[llvm] 3058245 - [NFC] Separate bitcode reading for FUNC_CODE_INST_CMPXCHG(_OLD)

Guillaume Chatelet via llvm-commits llvm-commits at lists.llvm.org
Sun Jul 12 00:55:39 PDT 2020


Hi Eric,

Thx a lot for the heads up and for the repro instructions! I'll have a look
soon.

Cheers,
Guillaume

On Sat, Jul 11, 2020 at 12:23 AM Eric Christopher <echristo at gmail.com>
wrote:

> Hi Guillaume,
>
> 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.
>
> Sorry for any inconvenience :(
>
> -eric
>
> On Thu, Jul 9, 2020 at 9:27 PM Guillaume Chatelet via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>>
>> Author: Guillaume Chatelet
>> Date: 2020-07-10T04:27:39Z
>> New Revision: 30582457b47004dec8a78144abc919a13ccbd08c
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/30582457b47004dec8a78144abc919a13ccbd08c
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/30582457b47004dec8a78144abc919a13ccbd08c.diff
>>
>> LOG: [NFC] Separate bitcode reading for FUNC_CODE_INST_CMPXCHG(_OLD)
>>
>> This is preparatory work to unable storing alignment for
>> AtomicCmpXchgInst.
>> See D83136 for context and bug:
>> https://bugs.llvm.org/show_bug.cgi?id=27168
>>
>> Differential Revision: https://reviews.llvm.org/D83375
>>
>> 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 de4fe6630324..a0c22a7d0905 100644
>> --- a/llvm/include/llvm/Bitcode/LLVMBitCodes.h
>> +++ b/llvm/include/llvm/Bitcode/LLVMBitCodes.h
>> @@ -536,8 +536,9 @@ 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,
>> align, vol,
>> -                                   //           ordering, synchscope]
>> +  FUNC_CODE_INST_CMPXCHG_OLD = 37, // CMPXCHG: [ptrty, ptr, cmp, new,
>> vol,
>> +                                   //           success_ordering, ssid,
>> +                                   //           failure_ordering?, weak?]
>>    FUNC_CODE_INST_ATOMICRMW = 38,   // ATOMICRMW: [ptrty,ptr,val,
>> operation,
>>                                     //             align, vol,
>>                                     //             ordering, synchscope]
>> @@ -551,8 +552,9 @@ 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,valty,cmp,new,
>> align,
>> -                                   //           vol,ordering,synchscope]
>> +  FUNC_CODE_INST_CMPXCHG = 46,     // CMPXCHG: [ptrty, ptr, cmp, newval,
>> vol,
>> +                                   //           success_ordering, ssid,
>> +                                   //           failure_ordering, weak]
>>    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 659e26c2bd25..ad1e97540298 100644
>> --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
>> +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
>> @@ -4982,63 +4982,120 @@ Error BitcodeReader::parseFunctionBody(Function
>> *F) {
>>        InstructionList.push_back(I);
>>        break;
>>      }
>> -    case bitc::FUNC_CODE_INST_CMPXCHG_OLD:
>> -    case bitc::FUNC_CODE_INST_CMPXCHG: {
>> -      // CMPXCHG:[ptrty, ptr, cmp, new, vol, successordering, ssid,
>> +    case bitc::FUNC_CODE_INST_CMPXCHG_OLD: {
>> +      // CMPXCHG:[ptrty, ptr, cmp, new, vol, success_ordering, ssid,
>>        //          failureordering?, isweak?]
>> -      unsigned OpNum = 0;
>> -      Value *Ptr, *Cmp, *New;
>> -      if (getValueTypePair(Record, OpNum, NextValueNo, Ptr, &FullTy))
>> +      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");
>>
>> -      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))
>> +      Value *Cmp = nullptr;
>> +      if (popValue(Record, Slot, NextValueNo,
>> getPointerElementFlatType(FullTy),
>> +                   Cmp))
>>          return error("Invalid record");
>> -      else
>> -        FullTy = cast<PointerType>(FullTy)->getElementType();
>>
>> -      if (popValue(Record, OpNum, NextValueNo, Cmp->getType(), New) ||
>> -          Record.size() < OpNum + 3 || Record.size() > OpNum + 5)
>> +      if (!(RecordCount == 6 || RecordCount == 7 || RecordCount == 8))
>>          return error("Invalid record");
>>
>> -      AtomicOrdering SuccessOrdering = getDecodedOrdering(Record[OpNum +
>> 1]);
>> -      if (SuccessOrdering == AtomicOrdering::NotAtomic ||
>> -          SuccessOrdering == AtomicOrdering::Unordered)
>> +      Value *New = nullptr;
>> +      if (popValue(Record, Slot, NextValueNo, Cmp->getType(), New))
>>          return error("Invalid record");
>> -      SyncScope::ID SSID = getDecodedSyncScopeID(Record[OpNum + 2]);
>>
>>        if (Error Err = typeCheckLoadStoreInst(Cmp->getType(),
>> Ptr->getType()))
>>          return Err;
>> +
>> +      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]);
>> +
>>        AtomicOrdering FailureOrdering;
>> -      if (Record.size() < 7)
>> +      if (RecordCount > 6)
>> +        FailureOrdering = getDecodedOrdering(Record[6]);
>> +      else
>>          FailureOrdering =
>>
>>  AtomicCmpXchgInst::getStrongestFailureOrdering(SuccessOrdering);
>> -      else
>> -        FailureOrdering = getDecodedOrdering(Record[OpNum + 3]);
>>
>> -      Align Alignment(
>> +      const 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]);
>>
>> -      if (Record.size() < 8) {
>> +      cast<AtomicCmpXchgInst>(I)->setVolatile(IsVol);
>> +
>> +      if (RecordCount > 7) {
>> +        cast<AtomicCmpXchgInst>(I)->setWeak(Record[7]);
>> +      } else {
>>          // 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;
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200712/efce3f4f/attachment.html>


More information about the llvm-commits mailing list