[PATCH] D140347: TargetLowering: Teach DemandedBits about VSCALE

[Benjamin Maxwell via Phabricator via llvm-commits]

MacDue added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:1136
+      return false;
+    APInt VScaleResultUpperbound(64, *MaxVScale);
+    VScaleResultUpperbound *= Op.getConstantOperandAPInt(0).sextOrTrunc(64);
----------------
compnerd wrote:
> craig.topper wrote:
> > I'm not sure that hardcoding 64 here is the right fix. I think what we need to check is that the multiply doesn't overflow.
> @craig.topper I absolutely agree with you.  I call this out in the commit message, that I'm just assuming that this is wide enough.  This can potentially overflow still.  Is there a good way to ensure that we have the proper width?
My thought was to do: 

```
    APInt Multiplier = Op.getConstantOperandAPInt(0);
    unsigned MultiplyBits = Log2_32(*MaxVScale) + 1 + Multiplier.getActiveBits();
    APInt VScaleResultUpperbound = APInt(MultiplyBits, *MaxVScale) * Multiplier.sextOrTrunc(MultiplyBits);
```
Which should always have enough bits to store the full result.


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

https://reviews.llvm.org/D140347