[PATCH] D128123: [SDAG] try to replace subtract-from-constant with xor

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D128123#3643672 <https://reviews.llvm.org/D128123#3643672>, @bjope wrote:

> Not sure really if this ends up as a regression or not, but with this source we can see some differences if for example using heaxagon or systemz as targets.
> (This example is related to doing sub->xor in instcombine so not exactly the patch in this review.)

Thanks for the example. The instcombine variation of this fold has a different problem that I noticed in another example.
Can you try the patch below and see if that fixes the regressions for your target?

  diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
  index 535a7736454c..c51ce0e8d1c3 100644
  --- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
  +++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
  @@ -1966,13 +1966,6 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
         return BinaryOperator::CreateAdd(X, ConstantExpr::getSub(C, C2));
     }
   
  -  // If there's no chance any bit will need to borrow from an adjacent bit:
  -  // sub C, X --> xor X, C
  -  const APInt *Op0C;
  -  if (match(Op0, m_APInt(Op0C)) &&
  -      (~computeKnownBits(Op1, 0, &I).Zero).isSubsetOf(*Op0C))
  -    return BinaryOperator::CreateXor(Op1, Op0);
  -
     {
       Value *Y;
       // X-(X+Y) == -Y    X-(Y+X) == -Y
  @@ -2231,7 +2224,20 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
           I, Builder.CreateIntrinsic(Intrinsic::ctpop, {I.getType()},
                                      {Builder.CreateNot(X)}));
   
  -  return TryToNarrowDeduceFlags();
  +  if (Instruction *R = TryToNarrowDeduceFlags())
  +    return R;
  +
  +  // If there's no chance any bit will need to borrow from an adjacent bit:
  +  // sub C, X --> xor X, C
  +  // Avoid this fold if the sub has no-wrap flags because that could be an
  +  // information-losing transform that we cannot recover from.
  +  const APInt *Op0C;
  +  if (!I.hasNoSignedWrap() && !I.hasNoUnsignedWrap() &&
  +      match(Op0, m_APInt(Op0C)) &&
  +      (~computeKnownBits(Op1, 0, &I).Zero).isSubsetOf(*Op0C))
  +    return BinaryOperator::CreateXor(Op1, Op0);
  +
  +  return nullptr;
   }
   
   /// This eliminates floating-point negation in either 'fneg(X)' or


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128123/new/

https://reviews.llvm.org/D128123