[llvm] cc28058 - Temporarily revert "[NFC] Separate bitcode reading for FUNC_CODE_INST_CMPXCHG(_OLD)"

Eric Christopher via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 10 15:21:10 PDT 2020


Author: Eric Christopher
Date: 2020-07-10T15:21:00-07:00
New Revision: cc28058c13e89ecc85dac7e1bd5d13a2ce1bb620

URL: https://github.com/llvm/llvm-project/commit/cc28058c13e89ecc85dac7e1bd5d13a2ce1bb620
DIFF: https://github.com/llvm/llvm-project/commit/cc28058c13e89ecc85dac7e1bd5d13a2ce1bb620.diff

LOG: 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.

Added: 
    

Modified: 
    llvm/include/llvm/Bitcode/LLVMBitCodes.h
    llvm/lib/Bitcode/Reader/BitcodeReader.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Bitcode/LLVMBitCodes.h b/llvm/include/llvm/Bitcode/LLVMBitCodes.h
index a0c22a7d0905..de4fe6630324 100644
--- a/llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ b/llvm/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/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index ad1e97540298..659e26c2bd25 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/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<PointerType>(Ptr->getType()))
         return error("Cmpxchg operand is not a pointer type");
 
-      Value *Cmp = nullptr;
-      if (popValue(Record, Slot, NextValueNo, getPointerElementFlatType(FullTy),
-                   Cmp))
-        return error("Invalid record");
-
-      if (!(RecordCount == 6 || RecordCount == 7 || RecordCount == 8))
+      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<PointerType>(FullTy)->getElementType();
 
-      Value *New = nullptr;
-      if (popValue(Record, Slot, NextValueNo, Cmp->getType(), New))
+      if (popValue(Record, OpNum, NextValueNo, Cmp->getType(), New) ||
+          Record.size() < OpNum + 3 || Record.size() > OpNum + 5)
         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<PointerType>(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<AtomicCmpXchgInst>(I)->setVolatile(Record[OpNum]);
 
-      cast<AtomicCmpXchgInst>(I)->setVolatile(IsVol);
-
-      if (RecordCount > 7) {
-        cast<AtomicCmpXchgInst>(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<StructType>(FullTy)->getElementType(0);
+      } else {
+        cast<AtomicCmpXchgInst>(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<PointerType>(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<AtomicCmpXchgInst>(I)->setVolatile(IsVol);
-      cast<AtomicCmpXchgInst>(I)->setWeak(IsWeak);
 
       InstructionList.push_back(I);
       break;


        


More information about the llvm-commits mailing list