[llvm] e5c5f92 - [InstCombine] switch synthetic unreachable to use undef instead of poison (NFC)

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 10 16:01:12 PDT 2022


I wouldn't bother for just a typo, but in this case, the commit message 
is actively confusing.  I'd spotted this and thought we'd hit another 
bug elsewhere and were working around something by deliberately moving 
back to undef.  It was only after reading the diff that I realized that 
was wrong.

Philip

On 6/10/22 15:02, Nuno Lopes wrote:
> Is that common procedure?
> I don't agree that that kind of noise is necessary (2 more commits). The patch is small enough that anyone reading it will understand the commit message got flipped.
>
> Nuno
>
> -----Original Message-----
> From: Philip Reames <listmail at philipreames.com>
> Sent: Friday, June 10, 2022 10:37 PM
> Subject: Re: [llvm] e5c5f92 - [InstCombine] switch synthetic unreachable to use undef instead of poison (NFC)
>
> Can you revert and reapply with a fixed message please?
>
> Philip
>
> On 6/10/22 14:35, Nuno Lopes wrote:
>> Ah yes, sorry. Replace undef with poison!
>>
>> Thanks,
>> Nuno
>>
>> -----Original Message-----
>> From: Philip Reames <listmail at philipreames.com>
>> Sent: Friday, June 10, 2022 10:16 PM
>> Subject: Re: [llvm] e5c5f92 - [InstCombine] switch synthetic
>> unreachable to use undef instead of poison (NFC)
>>
>> Did you swap the wording in the commit message here?
>>
>> On 6/10/22 13:54, Nuno Lopes via llvm-commits wrote:
>>> Author: Nuno Lopes
>>> Date: 2022-06-10T21:54:09+01:00
>>> New Revision: e5c5f92e12827fcf969002b1472b6ad9c08b1f14
>>>
>>> URL:
>>> https://github.com/llvm/llvm-project/commit/e5c5f92e12827fcf969002b14
>>> 7
>>> 2b6ad9c08b1f14
>>> DIFF:
>>> https://github.com/llvm/llvm-project/commit/e5c5f92e12827fcf969002b14
>>> 7
>>> 2b6ad9c08b1f14.diff
>>>
>>> LOG: [InstCombine] switch synthetic unreachable to use undef instead
>>> of poison (NFC)
>>>
>>> Added:
>>>        
>>>
>>> Modified:
>>>        llvm/lib/Transforms/InstCombine/InstCombineInternal.h
>>>        llvm/test/Transforms/InstCombine/element-atomic-memintrins.ll
>>>        llvm/test/Transforms/InstCombine/pr44245.ll
>>>
>>> Removed:
>>>        
>>>
>>>
>>> #####################################################################
>>> #
>>> ########## diff  --git
>>> a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
>>> b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
>>> index 6340c9996108..dcfc32ffb012 100644
>>> --- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
>>> +++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
>>> @@ -411,7 +411,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
>>>         // If we are replacing the instruction with itself, this must be in a
>>>         // segment of unreachable code, so just clobber the instruction.
>>>         if (&I == V)
>>> -      V = UndefValue::get(I.getType());
>>> +      V = PoisonValue::get(I.getType());
>>>     
>>>         LLVM_DEBUG(dbgs() << "IC: Replacing " << I << "\n"
>>>                           << "    with " << *V << '\n');
>>> @@ -439,7 +439,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
>>>       void CreateNonTerminatorUnreachable(Instruction *InsertAt) {
>>>         auto &Ctx = InsertAt->getContext();
>>>         new StoreInst(ConstantInt::getTrue(Ctx),
>>> -                  UndefValue::get(Type::getInt1PtrTy(Ctx)),
>>> +                  PoisonValue::get(Type::getInt1PtrTy(Ctx)),
>>>                       InsertAt);
>>>       }
>>>     
>>>
>>> diff  --git
>>> a/llvm/test/Transforms/InstCombine/element-atomic-memintrins.ll
>>> b/llvm/test/Transforms/InstCombine/element-atomic-memintrins.ll
>>> index 183e22e037d2..9e0d1ee376f1 100644
>>> --- a/llvm/test/Transforms/InstCombine/element-atomic-memintrins.ll
>>> +++ b/llvm/test/Transforms/InstCombine/element-atomic-memintrins.ll
>>> @@ -420,7 +420,7 @@ define void @test_undefined(i8* %dest, i8* %src, i1 %c1) {
>>>     ; CHECK-NEXT:  entry:
>>>     ; CHECK-NEXT:    br i1 [[C1:%.*]], label [[OK:%.*]], label [[UNDEFINED:%.*]]
>>>     ; CHECK:       undefined:
>>> -; CHECK-NEXT:    store i1 true, i1* undef, align 1
>>> +; CHECK-NEXT:    store i1 true, i1* poison, align 1
>>>     ; CHECK-NEXT:    br label [[OK]]
>>>     ; CHECK:       ok:
>>>     ; CHECK-NEXT:    ret void
>>>
>>> diff  --git a/llvm/test/Transforms/InstCombine/pr44245.ll
>>> b/llvm/test/Transforms/InstCombine/pr44245.ll
>>> index 1c123457383d..3ecd9028e923 100644
>>> --- a/llvm/test/Transforms/InstCombine/pr44245.ll
>>> +++ b/llvm/test/Transforms/InstCombine/pr44245.ll
>>> @@ -59,7 +59,7 @@ define void @test(i1 %c) {
>>>     ; CHECK-NEXT:    br label [[BB47]]
>>>     ; CHECK:       bb152:
>>>     ; CHECK-NEXT:    [[TMP1848]] = load i8*, i8** inttoptr (i64 16 to i8**), align 16
>>> -; CHECK-NEXT:    store i1 true, i1* undef, align 1
>>> +; CHECK-NEXT:    store i1 true, i1* poison, align 1
>>>     ; CHECK-NEXT:    br label [[BB150]]
>>>     ;
>>>     bb16:                                             ; preds = %bb
>>>
>>>
>>>            
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list