[PATCH] D140347: TargetLowering: Teach DemandedBits about VSCALE

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:1136
+      return false;
+    APInt VScaleResultUpperbound(64, *MaxVScale);
+    VScaleResultUpperbound *= Op.getConstantOperandAPInt(0).sextOrTrunc(64);
----------------
MacDue wrote:
> 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.
Here's what I was playing with

```
    APInt VScaleResultUpperbound(BitWidth, *MaxVScale);
    // TODO: Vscale max with no leading zeros requires special handling.
    if (VScaleResultUpperbound.isNegative())
      return false;
    bool Overflow;
    VScaleResultUpperbound =
        VScaleResultUpperbound.smul_ov(Op.getConstantOperandAPInt(0), Overflow);
    if (Overflow)
      return false;
    if (VScaleResultUpperbound.isNegative())
      Known.One.setHighBits(VScaleResultUpperbound.countLeadingOnes());
    else
      Known.Zero.setHighBits(VScaleResultUpperbound.countLeadingZeros());
```


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

https://reviews.llvm.org/D140347