From 206e1bb81187966be8d26f97a86d4b8c2b3891f8 Mon Sep 17 00:00:00 2001 From: Eric Christopher Date: Fri, 10 Jul 2020 15:10:52 -0700 Subject: [PATCH] Temporarily revert "[NFC] Separate bitcode reading for FUNC_CODE_INST_CMPXCHG(_OLD)" as it wasn't NFC and is causing issues with thinlto bitcode reading. I've followed up offline with reproduction instructions and testcases. This reverts commit 30582457b47004dec8a78144abc919a13ccbd08c. --- include/llvm/Bitcode/LLVMBitCodes.h | 10 +-- lib/Bitcode/Reader/BitcodeReader.cpp | 117 +++++++-------------------- 2 files changed, 34 insertions(+), 93 deletions(-) diff --git a/include/llvm/Bitcode/LLVMBitCodes.h b/include/llvm/Bitcode/LLVMBitCodes.h index a0c22a7d090..de4fe663032 100644 --- a/include/llvm/Bitcode/LLVMBitCodes.h +++ b/include/llvm/Bitcode/LLVMBitCodes.h @@ -536,9 +536,8 @@ enum FunctionCodes { FUNC_CODE_DEBUG_LOC = 35, // DEBUG_LOC: [Line,Col,ScopeVal, IAVal] FUNC_CODE_INST_FENCE = 36, // FENCE: [ordering, synchscope] - FUNC_CODE_INST_CMPXCHG_OLD = 37, // CMPXCHG: [ptrty, ptr, cmp, new, vol, - // success_ordering, ssid, - // failure_ordering?, weak?] + FUNC_CODE_INST_CMPXCHG_OLD = 37, // CMPXCHG: [ptrty,ptr,cmp,new, align, vol, + // ordering, synchscope] FUNC_CODE_INST_ATOMICRMW = 38, // ATOMICRMW: [ptrty,ptr,val, operation, // align, vol, // ordering, synchscope] @@ -552,9 +551,8 @@ enum FunctionCodes { FUNC_CODE_INST_GEP = 43, // GEP: [inbounds, n x operands] FUNC_CODE_INST_STORE = 44, // STORE: [ptrty,ptr,valty,val, align, vol] FUNC_CODE_INST_STOREATOMIC = 45, // STORE: [ptrty,ptr,val, align, vol - FUNC_CODE_INST_CMPXCHG = 46, // CMPXCHG: [ptrty, ptr, cmp, newval, vol, - // success_ordering, ssid, - // failure_ordering, weak] + FUNC_CODE_INST_CMPXCHG = 46, // CMPXCHG: [ptrty,ptr,valty,cmp,new, align, + // vol,ordering,synchscope] FUNC_CODE_INST_LANDINGPAD = 47, // LANDINGPAD: [ty,val,num,id0,val0...] FUNC_CODE_INST_CLEANUPRET = 48, // CLEANUPRET: [val] or [val,bb#] FUNC_CODE_INST_CATCHRET = 49, // CATCHRET: [val,bb#] diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index ad1e9754029..659e26c2bd2 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -4982,120 +4982,63 @@ Error BitcodeReader::parseFunctionBody(Function *F) { InstructionList.push_back(I); break; } - case bitc::FUNC_CODE_INST_CMPXCHG_OLD: { - // CMPXCHG:[ptrty, ptr, cmp, new, vol, success_ordering, ssid, + case bitc::FUNC_CODE_INST_CMPXCHG_OLD: + case bitc::FUNC_CODE_INST_CMPXCHG: { + // CMPXCHG:[ptrty, ptr, cmp, new, vol, successordering, ssid, // failureordering?, isweak?] - const size_t RecordCount = Record.size(); - unsigned Slot = 0; - Value *Ptr = nullptr; - if (getValueTypePair(Record, Slot, NextValueNo, Ptr, &FullTy)) + unsigned OpNum = 0; + Value *Ptr, *Cmp, *New; + if (getValueTypePair(Record, OpNum, NextValueNo, Ptr, &FullTy)) return error("Invalid record"); if (!isa(Ptr->getType())) return error("Cmpxchg operand is not a pointer type"); - Value *Cmp = nullptr; - if (popValue(Record, Slot, NextValueNo, getPointerElementFlatType(FullTy), - Cmp)) + if (BitCode == bitc::FUNC_CODE_INST_CMPXCHG) { + if (getValueTypePair(Record, OpNum, NextValueNo, Cmp, &FullTy)) + return error("Invalid record"); + } else if (popValue(Record, OpNum, NextValueNo, + getPointerElementFlatType(FullTy), Cmp)) + return error("Invalid record"); + else + FullTy = cast(FullTy)->getElementType(); + + if (popValue(Record, OpNum, NextValueNo, Cmp->getType(), New) || + Record.size() < OpNum + 3 || Record.size() > OpNum + 5) return error("Invalid record"); - if (!(RecordCount == 6 || RecordCount == 7 || RecordCount == 8)) - return error("Invalid record"); - - Value *New = nullptr; - if (popValue(Record, Slot, NextValueNo, Cmp->getType(), New)) - return error("Invalid record"); - - if (Error Err = typeCheckLoadStoreInst(Cmp->getType(), Ptr->getType())) - return Err; - - const bool IsVol = Record[3]; - - const AtomicOrdering SuccessOrdering = getDecodedOrdering(Record[4]); + AtomicOrdering SuccessOrdering = getDecodedOrdering(Record[OpNum + 1]); if (SuccessOrdering == AtomicOrdering::NotAtomic || SuccessOrdering == AtomicOrdering::Unordered) return error("Invalid record"); + SyncScope::ID SSID = getDecodedSyncScopeID(Record[OpNum + 2]); - const SyncScope::ID SSID = getDecodedSyncScopeID(Record[5]); - + if (Error Err = typeCheckLoadStoreInst(Cmp->getType(), Ptr->getType())) + return Err; AtomicOrdering FailureOrdering; - if (RecordCount > 6) - FailureOrdering = getDecodedOrdering(Record[6]); - else + if (Record.size() < 7) FailureOrdering = AtomicCmpXchgInst::getStrongestFailureOrdering(SuccessOrdering); + else + FailureOrdering = getDecodedOrdering(Record[OpNum + 3]); - const Align Alignment( + Align Alignment( TheModule->getDataLayout().getTypeStoreSize(Cmp->getType())); - - FullTy = cast(FullTy)->getElementType(); - FullTy = StructType::get(Context, {FullTy, Type::getInt1Ty(Context)}); I = new AtomicCmpXchgInst(Ptr, Cmp, New, Alignment, SuccessOrdering, FailureOrdering, SSID); + FullTy = StructType::get(Context, {FullTy, Type::getInt1Ty(Context)}); + cast(I)->setVolatile(Record[OpNum]); - cast(I)->setVolatile(IsVol); - - if (RecordCount > 7) { - cast(I)->setWeak(Record[7]); - } else { + if (Record.size() < 8) { // Before weak cmpxchgs existed, the instruction simply returned the // value loaded from memory, so bitcode files from that era will be // expecting the first component of a modern cmpxchg. CurBB->getInstList().push_back(I); I = ExtractValueInst::Create(I, 0); FullTy = cast(FullTy)->getElementType(0); + } else { + cast(I)->setWeak(Record[OpNum+4]); } - InstructionList.push_back(I); - break; - } - case bitc::FUNC_CODE_INST_CMPXCHG: { - // CMPXCHG: [ptrty, ptr, cmp, newval, vol, success_ordering, ssid, - // failure_ordering, weak] - const size_t RecordCount = Record.size(); - unsigned Slot = 0; - Value *Ptr = nullptr; - if (getValueTypePair(Record, Slot, NextValueNo, Ptr, &FullTy)) - return error("Invalid record"); - - if (!isa(Ptr->getType())) - return error("Cmpxchg operand is not a pointer type"); - - Value *Cmp = nullptr; - if (getValueTypePair(Record, Slot, NextValueNo, Cmp, &FullTy)) - return error("Invalid record"); - - if (RecordCount != 8) - return error("Invalid record"); - - Value *New = nullptr; - if (popValue(Record, Slot, NextValueNo, Cmp->getType(), New)) - return error("Invalid record"); - - const bool IsVol = Record[3]; - - const AtomicOrdering SuccessOrdering = getDecodedOrdering(Record[4]); - if (SuccessOrdering == AtomicOrdering::NotAtomic || - SuccessOrdering == AtomicOrdering::Unordered) - return error("Invalid record"); - - const SyncScope::ID SSID = getDecodedSyncScopeID(Record[5]); - - if (Error Err = typeCheckLoadStoreInst(Cmp->getType(), Ptr->getType())) - return Err; - - const AtomicOrdering FailureOrdering = getDecodedOrdering(Record[6]); - - const bool IsWeak = Record[7]; - - const Align Alignment( - TheModule->getDataLayout().getTypeStoreSize(Cmp->getType())); - - FullTy = StructType::get(Context, {FullTy, Type::getInt1Ty(Context)}); - I = new AtomicCmpXchgInst(Ptr, Cmp, New, Alignment, SuccessOrdering, - FailureOrdering, SSID); - - cast(I)->setVolatile(IsVol); - cast(I)->setWeak(IsWeak); InstructionList.push_back(I); break;