[PATCH] D108772: [LSR] Make sure that Factor fits into Base type

[Danila Malyutin via Phabricator via llvm-commits]

danilaml added inline comments.


================
Comment at: llvm/test/Transforms/LoopStrengthReduce/scaling-factor-incompat-type.ll:1
-; Check that it doesn't crash
+; Check that it doesn't crash, see pr42770
 ; REQUIRES: asserts
----------------
qcolombet wrote:
> Could you add a more descriptive comment on what this is testing and how? (I.e., the characteristics of the test and what we should be observing when the optimization does the right thing.)
It's more about preventing the wrong logic (that quickly leads to assert), when expecting anything out of the optimization.

In the test case Factor == 2^32 truncated to i16 would be 0, which meant that F.BaseRegs were zeroed out as well.
```
   const SCEV *FactorS = SE.getConstant(IntTy, Factor);

    // Check that multiplying with each base register doesn't overflow.
    for (size_t i = 0, e = F.BaseRegs.size(); i != e; ++i) {
      F.BaseRegs[i] = SE.getMulExpr(F.BaseRegs[i], FactorS)
```

This then quickly asserted in `InsertFormula`.

Anyway, hopefully if anyone needs more context, they can look up this review or bug report.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108772