[llvm] 8a9c70f - [InstCombine] C0 shift (X add nuw C) --> (C0 shift C) shift X
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 20 05:19:36 PDT 2022
Looking at the changelist and subsequent builds, this was not the breaking
change. It was a problem with this patch:
https://reviews.llvm.org/D123198
And that was reverted:
https://github.com/llvm/llvm-project/commit/14d93907213302dcd57b6dd2baeb9adccc59ab61
On Tue, Apr 19, 2022 at 7:56 PM Vitaly Buka <vitalybuka at google.com> wrote:
> Breaks the build https://lab.llvm.org/buildbot/#/builders/5/builds/22427
>
> On Tue, 19 Apr 2022 at 13:31, Sanjay Patel via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Good point! I removed those calls, and the existing tests for the fold
>> (just above the ones that I added) all still pass.
>>
>> The existing code was already strange to be calling isKnownNonNegative on
>> a constant.
>> I'll let this bake a bit then remove that clause and update the comment.
>>
>> On Tue, Apr 19, 2022 at 3:30 PM Nikita Popov <nikita.ppv at gmail.com>
>> wrote:
>>
>>> On Tue, Apr 19, 2022 at 9:21 PM Sanjay Patel via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>>
>>>> Author: Sanjay Patel
>>>> Date: 2022-04-19T15:21:34-04:00
>>>> New Revision: 8a9c70fc01e6c900f060f3c23d96ee444be33a9a
>>>>
>>>> URL:
>>>> https://github.com/llvm/llvm-project/commit/8a9c70fc01e6c900f060f3c23d96ee444be33a9a
>>>> DIFF:
>>>> https://github.com/llvm/llvm-project/commit/8a9c70fc01e6c900f060f3c23d96ee444be33a9a.diff
>>>>
>>>> LOG: [InstCombine] C0 shift (X add nuw C) --> (C0 shift C) shift X
>>>>
>>>> With 'nuw' we can convert the increment of the shift amount
>>>> into a pre-shift (constant fold) of the shifted constant:
>>>> https://alive2.llvm.org/ce/z/FkTyR2
>>>>
>>>> Fixes issue #41976
>>>>
>>>> Added:
>>>>
>>>>
>>>> Modified:
>>>> llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
>>>> llvm/test/Transforms/InstCombine/shift-add.ll
>>>>
>>>> Removed:
>>>>
>>>>
>>>>
>>>>
>>>> ################################################################################
>>>> diff --git a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
>>>> b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
>>>> index 26fe8e08d066d..cc4d53d27f031 100644
>>>> --- a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
>>>> +++ b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
>>>> @@ -399,13 +399,14 @@ Instruction
>>>> *InstCombinerImpl::commonShiftTransforms(BinaryOperator &I) {
>>>> reassociateShiftAmtsOfTwoSameDirectionShifts(&I, SQ)))
>>>> return NewShift;
>>>>
>>>> - // (C1 shift (A add C2)) -> (C1 shift C2) shift A)
>>>> - // iff A and C2 are both positive.
>>>> + // C0 shift (A add C) -> (C0 shift C) shift A
>>>> + // iff A and C2 are both positive or the add has 'nuw'.
>>>> Value *A;
>>>> Constant *C;
>>>> if (match(Op0, m_Constant()) && match(Op1, m_Add(m_Value(A),
>>>> m_Constant(C))))
>>>> - if (isKnownNonNegative(A, DL, 0, &AC, &I, &DT) &&
>>>> - isKnownNonNegative(C, DL, 0, &AC, &I, &DT))
>>>> + if (cast<BinaryOperator>(Op1)->hasNoUnsignedWrap() ||
>>>> + (isKnownNonNegative(A, DL, 0, &AC, &I, &DT) &&
>>>> + isKnownNonNegative(C, DL, 0, &AC, &I, &DT)))
>>>> return BinaryOperator::Create(
>>>> I.getOpcode(), Builder.CreateBinOp(I.getOpcode(), Op0, C),
>>>> A);
>>>>
>>>
>>> Just wondering, are those isKnownNonNegative() actually still necessary?
>>> I'd assume that we'd infer "add nuw" from those anyway.
>>>
>>> Nikita
>>>
>>>
>>>>
>>>> diff --git a/llvm/test/Transforms/InstCombine/shift-add.ll
>>>> b/llvm/test/Transforms/InstCombine/shift-add.ll
>>>> index d9a1c3b1878b3..f75f6a95f8f8a 100644
>>>> --- a/llvm/test/Transforms/InstCombine/shift-add.ll
>>>> +++ b/llvm/test/Transforms/InstCombine/shift-add.ll
>>>> @@ -123,8 +123,7 @@ define <4 x i32> @lshr_C1_add_A_C2_v4i32_splat(i16
>>>> %I) {
>>>>
>>>> define i32 @shl_add_nuw(i32 %x) {
>>>> ; CHECK-LABEL: @shl_add_nuw(
>>>> -; CHECK-NEXT: [[A:%.*]] = add nuw i32 [[X:%.*]], 5
>>>> -; CHECK-NEXT: [[R:%.*]] = shl i32 6, [[A]]
>>>> +; CHECK-NEXT: [[R:%.*]] = shl i32 192, [[X:%.*]]
>>>> ; CHECK-NEXT: ret i32 [[R]]
>>>> ;
>>>> %a = add nuw i32 %x, 5
>>>> @@ -132,10 +131,11 @@ define i32 @shl_add_nuw(i32 %x) {
>>>> ret i32 %r
>>>> }
>>>>
>>>> +; vectors with arbitrary constants work too
>>>> +
>>>> define <2 x i12> @lshr_add_nuw(<2 x i12> %x) {
>>>> ; CHECK-LABEL: @lshr_add_nuw(
>>>> -; CHECK-NEXT: [[A:%.*]] = add nuw <2 x i12> [[X:%.*]], <i12 5, i12
>>>> 1>
>>>> -; CHECK-NEXT: [[R:%.*]] = lshr <2 x i12> <i12 6, i12 42>, [[A]]
>>>> +; CHECK-NEXT: [[R:%.*]] = lshr <2 x i12> <i12 0, i12 21>, [[X:%.*]]
>>>> ; CHECK-NEXT: ret <2 x i12> [[R]]
>>>> ;
>>>> %a = add nuw <2 x i12> %x, <i12 5, i12 1>
>>>> @@ -143,12 +143,13 @@ define <2 x i12> @lshr_add_nuw(<2 x i12> %x) {
>>>> ret <2 x i12> %r
>>>> }
>>>>
>>>> +; extra use is ok and in this case the result can be simplified to a
>>>> constant
>>>> +
>>>> define i32 @ashr_add_nuw(i32 %x, i32* %p) {
>>>> ; CHECK-LABEL: @ashr_add_nuw(
>>>> ; CHECK-NEXT: [[A:%.*]] = add nuw i32 [[X:%.*]], 5
>>>> ; CHECK-NEXT: store i32 [[A]], i32* [[P:%.*]], align 4
>>>> -; CHECK-NEXT: [[R:%.*]] = ashr i32 -6, [[A]]
>>>> -; CHECK-NEXT: ret i32 [[R]]
>>>> +; CHECK-NEXT: ret i32 -1
>>>> ;
>>>> %a = add nuw i32 %x, 5
>>>> store i32 %a, i32* %p
>>>> @@ -156,6 +157,8 @@ define i32 @ashr_add_nuw(i32 %x, i32* %p) {
>>>> ret i32 %r
>>>> }
>>>>
>>>> +; negative test - must have 'nuw'
>>>> +
>>>> define i32 @shl_add_nsw(i32 %x) {
>>>> ; CHECK-LABEL: @shl_add_nsw(
>>>> ; CHECK-NEXT: [[A:%.*]] = add nsw i32 [[X:%.*]], 5
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>> _______________________________________________
>> 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/20220420/edbc082e/attachment.html>
More information about the llvm-commits
mailing list