<div dir="ltr">Hi Eric,<div><br></div><div>Thx a lot for the heads up and for the repro instructions! I'll have a look soon.</div><div><br></div><div>Cheers,</div><div>Guillaume</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Jul 11, 2020 at 12:23 AM Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi Guillaume,<div><br></div><div>I've temporarily reverted this in cc28058c13e89ecc85dac7e1bd5d13a2ce1bb620 as it was causing compilation errors for thinlto. I'll send you an email with some reproduction instructions offline.</div><div><br></div><div>Sorry for any inconvenience :(</div><div><br></div><div>-eric</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jul 9, 2020 at 9:27 PM Guillaume Chatelet via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: Guillaume Chatelet<br>
Date: 2020-07-10T04:27:39Z<br>
New Revision: 30582457b47004dec8a78144abc919a13ccbd08c<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/30582457b47004dec8a78144abc919a13ccbd08c" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/30582457b47004dec8a78144abc919a13ccbd08c</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/30582457b47004dec8a78144abc919a13ccbd08c.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/30582457b47004dec8a78144abc919a13ccbd08c.diff</a><br>
<br>
LOG: [NFC] Separate bitcode reading for FUNC_CODE_INST_CMPXCHG(_OLD)<br>
<br>
This is preparatory work to unable storing alignment for AtomicCmpXchgInst.<br>
See D83136 for context and bug: <a href="https://bugs.llvm.org/show_bug.cgi?id=27168" rel="noreferrer" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=27168</a><br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D83375" rel="noreferrer" target="_blank">https://reviews.llvm.org/D83375</a><br>
<br>
Added: <br>
<br>
<br>
Modified: <br>
    llvm/include/llvm/Bitcode/LLVMBitCodes.h<br>
    llvm/lib/Bitcode/Reader/BitcodeReader.cpp<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff  --git a/llvm/include/llvm/Bitcode/LLVMBitCodes.h b/llvm/include/llvm/Bitcode/LLVMBitCodes.h<br>
index de4fe6630324..a0c22a7d0905 100644<br>
--- a/llvm/include/llvm/Bitcode/LLVMBitCodes.h<br>
+++ b/llvm/include/llvm/Bitcode/LLVMBitCodes.h<br>
@@ -536,8 +536,9 @@ enum FunctionCodes {<br>
<br>
   FUNC_CODE_DEBUG_LOC = 35,        // DEBUG_LOC:  [Line,Col,ScopeVal, IAVal]<br>
   FUNC_CODE_INST_FENCE = 36,       // FENCE: [ordering, synchscope]<br>
-  FUNC_CODE_INST_CMPXCHG_OLD = 37, // CMPXCHG: [ptrty,ptr,cmp,new, align, vol,<br>
-                                   //           ordering, synchscope]<br>
+  FUNC_CODE_INST_CMPXCHG_OLD = 37, // CMPXCHG: [ptrty, ptr, cmp, new, vol,<br>
+                                   //           success_ordering, ssid,<br>
+                                   //           failure_ordering?, weak?]<br>
   FUNC_CODE_INST_ATOMICRMW = 38,   // ATOMICRMW: [ptrty,ptr,val, operation,<br>
                                    //             align, vol,<br>
                                    //             ordering, synchscope]<br>
@@ -551,8 +552,9 @@ enum FunctionCodes {<br>
   FUNC_CODE_INST_GEP = 43,             // GEP:  [inbounds, n x operands]<br>
   FUNC_CODE_INST_STORE = 44,       // STORE: [ptrty,ptr,valty,val, align, vol]<br>
   FUNC_CODE_INST_STOREATOMIC = 45, // STORE: [ptrty,ptr,val, align, vol<br>
-  FUNC_CODE_INST_CMPXCHG = 46,     // CMPXCHG: [ptrty,ptr,valty,cmp,new, align,<br>
-                                   //           vol,ordering,synchscope]<br>
+  FUNC_CODE_INST_CMPXCHG = 46,     // CMPXCHG: [ptrty, ptr, cmp, newval, vol,<br>
+                                   //           success_ordering, ssid,<br>
+                                   //           failure_ordering, weak]<br>
   FUNC_CODE_INST_LANDINGPAD = 47,  // LANDINGPAD: [ty,val,num,id0,val0...]<br>
   FUNC_CODE_INST_CLEANUPRET = 48,  // CLEANUPRET: [val] or [val,bb#]<br>
   FUNC_CODE_INST_CATCHRET = 49,    // CATCHRET: [val,bb#]<br>
<br>
diff  --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp<br>
index 659e26c2bd25..ad1e97540298 100644<br>
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp<br>
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp<br>
@@ -4982,63 +4982,120 @@ Error BitcodeReader::parseFunctionBody(Function *F) {<br>
       InstructionList.push_back(I);<br>
       break;<br>
     }<br>
-    case bitc::FUNC_CODE_INST_CMPXCHG_OLD:<br>
-    case bitc::FUNC_CODE_INST_CMPXCHG: {<br>
-      // CMPXCHG:[ptrty, ptr, cmp, new, vol, successordering, ssid,<br>
+    case bitc::FUNC_CODE_INST_CMPXCHG_OLD: {<br>
+      // CMPXCHG:[ptrty, ptr, cmp, new, vol, success_ordering, ssid,<br>
       //          failureordering?, isweak?]<br>
-      unsigned OpNum = 0;<br>
-      Value *Ptr, *Cmp, *New;<br>
-      if (getValueTypePair(Record, OpNum, NextValueNo, Ptr, &FullTy))<br>
+      const size_t RecordCount = Record.size();<br>
+      unsigned Slot = 0;<br>
+      Value *Ptr = nullptr;<br>
+      if (getValueTypePair(Record, Slot, NextValueNo, Ptr, &FullTy))<br>
         return error("Invalid record");<br>
<br>
       if (!isa<PointerType>(Ptr->getType()))<br>
         return error("Cmpxchg operand is not a pointer type");<br>
<br>
-      if (BitCode == bitc::FUNC_CODE_INST_CMPXCHG) {<br>
-        if (getValueTypePair(Record, OpNum, NextValueNo, Cmp, &FullTy))<br>
-          return error("Invalid record");<br>
-      } else if (popValue(Record, OpNum, NextValueNo,<br>
-                          getPointerElementFlatType(FullTy), Cmp))<br>
+      Value *Cmp = nullptr;<br>
+      if (popValue(Record, Slot, NextValueNo, getPointerElementFlatType(FullTy),<br>
+                   Cmp))<br>
         return error("Invalid record");<br>
-      else<br>
-        FullTy = cast<PointerType>(FullTy)->getElementType();<br>
<br>
-      if (popValue(Record, OpNum, NextValueNo, Cmp->getType(), New) ||<br>
-          Record.size() < OpNum + 3 || Record.size() > OpNum + 5)<br>
+      if (!(RecordCount == 6 || RecordCount == 7 || RecordCount == 8))<br>
         return error("Invalid record");<br>
<br>
-      AtomicOrdering SuccessOrdering = getDecodedOrdering(Record[OpNum + 1]);<br>
-      if (SuccessOrdering == AtomicOrdering::NotAtomic ||<br>
-          SuccessOrdering == AtomicOrdering::Unordered)<br>
+      Value *New = nullptr;<br>
+      if (popValue(Record, Slot, NextValueNo, Cmp->getType(), New))<br>
         return error("Invalid record");<br>
-      SyncScope::ID SSID = getDecodedSyncScopeID(Record[OpNum + 2]);<br>
<br>
       if (Error Err = typeCheckLoadStoreInst(Cmp->getType(), Ptr->getType()))<br>
         return Err;<br>
+<br>
+      const bool IsVol = Record[3];<br>
+<br>
+      const AtomicOrdering SuccessOrdering = getDecodedOrdering(Record[4]);<br>
+      if (SuccessOrdering == AtomicOrdering::NotAtomic ||<br>
+          SuccessOrdering == AtomicOrdering::Unordered)<br>
+        return error("Invalid record");<br>
+<br>
+      const SyncScope::ID SSID = getDecodedSyncScopeID(Record[5]);<br>
+<br>
       AtomicOrdering FailureOrdering;<br>
-      if (Record.size() < 7)<br>
+      if (RecordCount > 6)<br>
+        FailureOrdering = getDecodedOrdering(Record[6]);<br>
+      else<br>
         FailureOrdering =<br>
             AtomicCmpXchgInst::getStrongestFailureOrdering(SuccessOrdering);<br>
-      else<br>
-        FailureOrdering = getDecodedOrdering(Record[OpNum + 3]);<br>
<br>
-      Align Alignment(<br>
+      const Align Alignment(<br>
           TheModule->getDataLayout().getTypeStoreSize(Cmp->getType()));<br>
+<br>
+      FullTy = cast<PointerType>(FullTy)->getElementType();<br>
+      FullTy = StructType::get(Context, {FullTy, Type::getInt1Ty(Context)});<br>
       I = new AtomicCmpXchgInst(Ptr, Cmp, New, Alignment, SuccessOrdering,<br>
                                 FailureOrdering, SSID);<br>
-      FullTy = StructType::get(Context, {FullTy, Type::getInt1Ty(Context)});<br>
-      cast<AtomicCmpXchgInst>(I)->setVolatile(Record[OpNum]);<br>
<br>
-      if (Record.size() < 8) {<br>
+      cast<AtomicCmpXchgInst>(I)->setVolatile(IsVol);<br>
+<br>
+      if (RecordCount > 7) {<br>
+        cast<AtomicCmpXchgInst>(I)->setWeak(Record[7]);<br>
+      } else {<br>
         // Before weak cmpxchgs existed, the instruction simply returned the<br>
         // value loaded from memory, so bitcode files from that era will be<br>
         // expecting the first component of a modern cmpxchg.<br>
         CurBB->getInstList().push_back(I);<br>
         I = ExtractValueInst::Create(I, 0);<br>
         FullTy = cast<StructType>(FullTy)->getElementType(0);<br>
-      } else {<br>
-        cast<AtomicCmpXchgInst>(I)->setWeak(Record[OpNum+4]);<br>
       }<br>
+      InstructionList.push_back(I);<br>
+      break;<br>
+    }<br>
+    case bitc::FUNC_CODE_INST_CMPXCHG: {<br>
+      // CMPXCHG: [ptrty, ptr, cmp, newval, vol, success_ordering, ssid,<br>
+      //           failure_ordering, weak]<br>
+      const size_t RecordCount = Record.size();<br>
+      unsigned Slot = 0;<br>
+      Value *Ptr = nullptr;<br>
+      if (getValueTypePair(Record, Slot, NextValueNo, Ptr, &FullTy))<br>
+        return error("Invalid record");<br>
+<br>
+      if (!isa<PointerType>(Ptr->getType()))<br>
+        return error("Cmpxchg operand is not a pointer type");<br>
+<br>
+      Value *Cmp = nullptr;<br>
+      if (getValueTypePair(Record, Slot, NextValueNo, Cmp, &FullTy))<br>
+        return error("Invalid record");<br>
+<br>
+      if (RecordCount != 8)<br>
+        return error("Invalid record");<br>
+<br>
+      Value *New = nullptr;<br>
+      if (popValue(Record, Slot, NextValueNo, Cmp->getType(), New))<br>
+        return error("Invalid record");<br>
+<br>
+      const bool IsVol = Record[3];<br>
+<br>
+      const AtomicOrdering SuccessOrdering = getDecodedOrdering(Record[4]);<br>
+      if (SuccessOrdering == AtomicOrdering::NotAtomic ||<br>
+          SuccessOrdering == AtomicOrdering::Unordered)<br>
+        return error("Invalid record");<br>
+<br>
+      const SyncScope::ID SSID = getDecodedSyncScopeID(Record[5]);<br>
+<br>
+      if (Error Err = typeCheckLoadStoreInst(Cmp->getType(), Ptr->getType()))<br>
+        return Err;<br>
+<br>
+      const AtomicOrdering FailureOrdering = getDecodedOrdering(Record[6]);<br>
+<br>
+      const bool IsWeak = Record[7];<br>
+<br>
+      const Align Alignment(<br>
+          TheModule->getDataLayout().getTypeStoreSize(Cmp->getType()));<br>
+<br>
+      FullTy = StructType::get(Context, {FullTy, Type::getInt1Ty(Context)});<br>
+      I = new AtomicCmpXchgInst(Ptr, Cmp, New, Alignment, SuccessOrdering,<br>
+                                FailureOrdering, SSID);<br>
+<br>
+      cast<AtomicCmpXchgInst>(I)->setVolatile(IsVol);<br>
+      cast<AtomicCmpXchgInst>(I)->setWeak(IsWeak);<br>
<br>
       InstructionList.push_back(I);<br>
       break;<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>
</blockquote></div>