[PATCH] D105009: [LSR] Handle case 1*reg => reg. PR50918

[Huihui Zhang via Phabricator via llvm-commits]

huihuiz added a comment.

Another note on LSRInstance::GenerateReassociationsImpl() where 1*reg is created, eventually trigger this assertion.

Looks like LSR is rewriting Formula

  reg((-64 + (32 * ((112 * (((trunc i32 undef to i8) * ((trunc i32 %tmp3 to i8) + {94,+,-3}<%bb1>)) + {-94,+,3}<%bb1>)) + (-56 * (trunc i32 %tmp3 to i8)) + {-110,+,88}<%bb1>)))) + 1*reg({0,+,-128}<%bb21>)

into

  reg({0,+,-128}<%bb21>) + imm(256)

I am not sure this rewrite is correct ?



================
Comment at: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp:488-489
 
-  if (Scale == 1 && BaseRegs.empty())
-    return false;
-
----------------
We will still need to keep this logic unchanged.

BaseRegs could be erased in LSRInstance::GenerateReassociationsImpl(), it's possible to have a non-canonical formula 1*reg, which need to be canonicalized later on.


```
    if (InnerSumSC && SE.getTypeSizeInBits(InnerSumSC->getType()) <= 64 &&
        TTI.isLegalAddImmediate((uint64_t)F.UnfoldedOffset +
                                InnerSumSC->getValue()->getZExtValue())) {
      F.UnfoldedOffset =
          (uint64_t)F.UnfoldedOffset + InnerSumSC->getValue()->getZExtValue();
      if (IsScaledReg)
        F.ScaledReg = nullptr;
      else
        // This may create a non-canonical formula 1*reg
        F.BaseRegs.erase(F.BaseRegs.begin() + Idx);  
    } else if (IsScaledReg)
      F.ScaledReg = InnerSum;
    else
      F.BaseRegs[Idx] = InnerSum;

```


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

https://reviews.llvm.org/D105009