[llvm] r290733 - [InstCombine] More thoroughly canonicalize the position of zexts

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 12 14:29:10 PST 2017


Oh, sorry I missed that one.

I was rummaging around for shrinking transforms and noticed the 'add'
transforms added here. When I commented out the 1st block under:
  // Check for (add (sext x), y), see if we can merge this into an

and:
  // Check for (add (zext x), y), see if we can merge this into an

...nothing failed. I don't think there's any test coverage for the
transforms with a constant RHS?

Something like this:
define i64 @test2b(i32 %V) {
  %call2 = call i32 @callee(), !range !0
  %zext = sext i32 %call2 to i64
  %add = add i64 %zext, 19
  ret i64 %add
}



On Thu, Jan 12, 2017 at 2:40 PM, David Majnemer <david.majnemer at gmail.com>
wrote:

> Yes, I added them in r290929. They were accidentally omitted in the
> original commit.
>
> On Thu, Jan 12, 2017 at 1:33 PM, Sanjay Patel <spatel at rotateright.com>
> wrote:
>
>> Have any tests been added for these transforms?
>>
>> On Thu, Dec 29, 2016 at 8:47 PM, David Majnemer via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Addressed in r290741, thanks.
>>>
>>> On Thu, Dec 29, 2016 at 5:18 PM, Craig Topper <craig.topper at gmail.com>
>>> wrote:
>>>
>>>> Why does (add (zext x), cst) --> (zext (add x, cst')) check for signed
>>>> overflow but then create an NUW add. That's not valid is it?
>>>>
>>>> ~Craig
>>>>
>>>> On Thu, Dec 29, 2016 at 4:28 PM, David Majnemer via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> Author: majnemer
>>>>> Date: Thu Dec 29 18:28:58 2016
>>>>> New Revision: 290733
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=290733&view=rev
>>>>> Log:
>>>>> [InstCombine] More thoroughly canonicalize the position of zexts
>>>>>
>>>>> We correctly canonicalized (add (sext x), (sext y)) to (sext (add x,
>>>>> y))
>>>>> where possible.  However, we didn't perform the same canonicalization
>>>>> for zexts or for muls.
>>>>>
>>>>> Modified:
>>>>>     llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
>>>>>     llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
>>>>>
>>>>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transform
>>>>> s/InstCombine/InstCombineAddSub.cpp?rev=290733&r1=290732&r2=
>>>>> 290733&view=diff
>>>>> ============================================================
>>>>> ==================
>>>>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
>>>>> (original)
>>>>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp Thu
>>>>> Dec 29 18:28:58 2016
>>>>> @@ -1226,15 +1226,16 @@ Instruction *InstCombiner::visitAdd(Bina
>>>>>    if (SExtInst *LHSConv = dyn_cast<SExtInst>(LHS)) {
>>>>>      // (add (sext x), cst) --> (sext (add x, cst'))
>>>>>      if (ConstantInt *RHSC = dyn_cast<ConstantInt>(RHS)) {
>>>>> -      Constant *CI =
>>>>> -        ConstantExpr::getTrunc(RHSC, LHSConv->getOperand(0)->getTyp
>>>>> e());
>>>>> -      if (LHSConv->hasOneUse() &&
>>>>> -          ConstantExpr::getSExt(CI, I.getType()) == RHSC &&
>>>>> -          WillNotOverflowSignedAdd(LHSConv->getOperand(0), CI, I)) {
>>>>> -        // Insert the new, smaller add.
>>>>> -        Value *NewAdd = Builder->CreateNSWAdd(LHSConv->getOperand(0),
>>>>> -                                              CI, "addconv");
>>>>> -        return new SExtInst(NewAdd, I.getType());
>>>>> +      if (LHSConv->hasOneUse()) {
>>>>> +        Constant *CI =
>>>>> +            ConstantExpr::getTrunc(RHSC,
>>>>> LHSConv->getOperand(0)->getType());
>>>>> +        if (ConstantExpr::getSExt(CI, I.getType()) == RHSC &&
>>>>> +            WillNotOverflowSignedAdd(LHSConv->getOperand(0), CI, I))
>>>>> {
>>>>> +          // Insert the new, smaller add.
>>>>> +          Value *NewAdd =
>>>>> +              Builder->CreateNSWAdd(LHSConv->getOperand(0), CI,
>>>>> "addconv");
>>>>> +          return new SExtInst(NewAdd, I.getType());
>>>>> +        }
>>>>>        }
>>>>>      }
>>>>>
>>>>> @@ -1255,6 +1256,43 @@ Instruction *InstCombiner::visitAdd(Bina
>>>>>        }
>>>>>      }
>>>>>    }
>>>>> +
>>>>> +  // Check for (add (zext x), y), see if we can merge this into an
>>>>> +  // integer add followed by a zext.
>>>>> +  if (auto *LHSConv = dyn_cast<ZExtInst>(LHS)) {
>>>>> +    // (add (zext x), cst) --> (zext (add x, cst'))
>>>>> +    if (ConstantInt *RHSC = dyn_cast<ConstantInt>(RHS)) {
>>>>> +      if (LHSConv->hasOneUse()) {
>>>>> +        Constant *CI =
>>>>> +            ConstantExpr::getTrunc(RHSC,
>>>>> LHSConv->getOperand(0)->getType());
>>>>> +        if (ConstantExpr::getZExt(CI, I.getType()) == RHSC &&
>>>>> +            WillNotOverflowSignedAdd(LHSConv->getOperand(0), CI, I))
>>>>> {
>>>>> +          // Insert the new, smaller add.
>>>>> +          Value *NewAdd =
>>>>> +              Builder->CreateNUWAdd(LHSConv->getOperand(0), CI,
>>>>> "addconv");
>>>>> +          return new ZExtInst(NewAdd, I.getType());
>>>>> +        }
>>>>> +      }
>>>>> +    }
>>>>> +
>>>>> +    // (add (zext x), (zext y)) --> (zext (add int x, y))
>>>>> +    if (auto *RHSConv = dyn_cast<ZExtInst>(RHS)) {
>>>>> +      // Only do this if x/y have the same type, if at last one of
>>>>> them has a
>>>>> +      // single use (so we don't increase the number of zexts), and
>>>>> if the
>>>>> +      // integer add will not overflow.
>>>>> +      if (LHSConv->getOperand(0)->getType() ==
>>>>> +              RHSConv->getOperand(0)->getType() &&
>>>>> +          (LHSConv->hasOneUse() || RHSConv->hasOneUse()) &&
>>>>> +          computeOverflowForUnsignedAdd(LHSConv->getOperand(0),
>>>>> +                                        RHSConv->getOperand(0),
>>>>> +                                        &I) ==
>>>>> OverflowResult::NeverOverflows) {
>>>>> +        // Insert the new integer add.
>>>>> +        Value *NewAdd = Builder->CreateNUWAdd(
>>>>> +            LHSConv->getOperand(0), RHSConv->getOperand(0),
>>>>> "addconv");
>>>>> +        return new ZExtInst(NewAdd, I.getType());
>>>>> +      }
>>>>> +    }
>>>>> +  }
>>>>>
>>>>>    // (add (xor A, B) (and A, B)) --> (or A, B)
>>>>>    {
>>>>>
>>>>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.c
>>>>> pp
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transform
>>>>> s/InstCombine/InstCombineMulDivRem.cpp?rev=290733&r1=290732&
>>>>> r2=290733&view=diff
>>>>> ============================================================
>>>>> ==================
>>>>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
>>>>> (original)
>>>>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
>>>>> Thu Dec 29 18:28:58 2016
>>>>> @@ -389,6 +389,79 @@ Instruction *InstCombiner::visitMul(Bina
>>>>>      }
>>>>>    }
>>>>>
>>>>> +  // Check for (mul (sext x), y), see if we can merge this into an
>>>>> +  // integer mul followed by a sext.
>>>>> +  if (SExtInst *Op0Conv = dyn_cast<SExtInst>(Op0)) {
>>>>> +    // (mul (sext x), cst) --> (sext (mul x, cst'))
>>>>> +    if (ConstantInt *Op1C = dyn_cast<ConstantInt>(Op1)) {
>>>>> +      if (Op0Conv->hasOneUse()) {
>>>>> +        Constant *CI =
>>>>> +            ConstantExpr::getTrunc(Op1C,
>>>>> Op0Conv->getOperand(0)->getType());
>>>>> +        if (ConstantExpr::getSExt(CI, I.getType()) == Op1C &&
>>>>> +            WillNotOverflowSignedMul(Op0Conv->getOperand(0), CI, I))
>>>>> {
>>>>> +          // Insert the new, smaller mul.
>>>>> +          Value *NewMul =
>>>>> +              Builder->CreateNSWMul(Op0Conv->getOperand(0), CI,
>>>>> "mulconv");
>>>>> +          return new SExtInst(NewMul, I.getType());
>>>>> +        }
>>>>> +      }
>>>>> +    }
>>>>> +
>>>>> +    // (mul (sext x), (sext y)) --> (sext (mul int x, y))
>>>>> +    if (SExtInst *Op1Conv = dyn_cast<SExtInst>(Op1)) {
>>>>> +      // Only do this if x/y have the same type, if at last one of
>>>>> them has a
>>>>> +      // single use (so we don't increase the number of sexts), and
>>>>> if the
>>>>> +      // integer mul will not overflow.
>>>>> +      if (Op0Conv->getOperand(0)->getType() ==
>>>>> +              Op1Conv->getOperand(0)->getType() &&
>>>>> +          (Op0Conv->hasOneUse() || Op1Conv->hasOneUse()) &&
>>>>> +          WillNotOverflowSignedMul(Op0Conv->getOperand(0),
>>>>> +                                   Op1Conv->getOperand(0), I)) {
>>>>> +        // Insert the new integer mul.
>>>>> +        Value *NewMul = Builder->CreateNSWMul(
>>>>> +            Op0Conv->getOperand(0), Op1Conv->getOperand(0),
>>>>> "mulconv");
>>>>> +        return new SExtInst(NewMul, I.getType());
>>>>> +      }
>>>>> +    }
>>>>> +  }
>>>>> +
>>>>> +  // Check for (mul (zext x), y), see if we can merge this into an
>>>>> +  // integer mul followed by a zext.
>>>>> +  if (auto *Op0Conv = dyn_cast<ZExtInst>(Op0)) {
>>>>> +    // (mul (zext x), cst) --> (zext (mul x, cst'))
>>>>> +    if (ConstantInt *Op1C = dyn_cast<ConstantInt>(Op1)) {
>>>>> +      if (Op0Conv->hasOneUse()) {
>>>>> +        Constant *CI =
>>>>> +            ConstantExpr::getTrunc(Op1C,
>>>>> Op0Conv->getOperand(0)->getType());
>>>>> +        if (ConstantExpr::getZExt(CI, I.getType()) == Op1C &&
>>>>> +            WillNotOverflowSignedMul(Op0Conv->getOperand(0), CI, I))
>>>>> {
>>>>> +          // Insert the new, smaller mul.
>>>>> +          Value *NewMul =
>>>>> +              Builder->CreateNUWMul(Op0Conv->getOperand(0), CI,
>>>>> "mulconv");
>>>>> +          return new ZExtInst(NewMul, I.getType());
>>>>> +        }
>>>>> +      }
>>>>> +    }
>>>>> +
>>>>> +    // (mul (zext x), (zext y)) --> (zext (mul int x, y))
>>>>> +    if (auto *Op1Conv = dyn_cast<ZExtInst>(Op1)) {
>>>>> +      // Only do this if x/y have the same type, if at last one of
>>>>> them has a
>>>>> +      // single use (so we don't increase the number of zexts), and
>>>>> if the
>>>>> +      // integer mul will not overflow.
>>>>> +      if (Op0Conv->getOperand(0)->getType() ==
>>>>> +              Op1Conv->getOperand(0)->getType() &&
>>>>> +          (Op0Conv->hasOneUse() || Op1Conv->hasOneUse()) &&
>>>>> +          computeOverflowForUnsignedMul(Op0Conv->getOperand(0),
>>>>> +                                        Op1Conv->getOperand(0),
>>>>> +                                        &I) ==
>>>>> OverflowResult::NeverOverflows) {
>>>>> +        // Insert the new integer mul.
>>>>> +        Value *NewMul = Builder->CreateNUWMul(
>>>>> +            Op0Conv->getOperand(0), Op1Conv->getOperand(0),
>>>>> "mulconv");
>>>>> +        return new ZExtInst(NewMul, I.getType());
>>>>> +      }
>>>>> +    }
>>>>> +  }
>>>>> +
>>>>>    if (!I.hasNoSignedWrap() && WillNotOverflowSignedMul(Op0, Op1, I)) {
>>>>>      Changed = true;
>>>>>      I.setHasNoSignedWrap(true);
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>
>>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://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/20170112/b4b5a6cf/attachment.html>


More information about the llvm-commits mailing list