[PATCH] D109883: [Analysis] Add support for vscale in computeKnownBitsFromOperator

Fri Sep 17 01:01:06 PDT 2021

dmgreen added inline comments.

================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:1705
+        unsigned FirstZeroBit = 64 - countLeadingZeros(MaxVScale);
+        unsigned VScaleTypeSize = Q.DL.getTypeSizeInBits(II->getType()->getScalarType());
+        if (FirstZeroBit < VScaleTypeSize)
----------------
david-arm wrote:
> dmgreen wrote:
> > Will VScaleTypeSize be BitWidth?
> I can't see anything in the code that guarantees the result type of an instruction will have the same width as Known. However, I'm not sure this really matters? If VScaleTypeSize < BitWidth, then I'd expect any bits above BitWidth to also be zero. I looked at Instruction::ZExt as a guide just now and we first truncate Known to be the same as the input, compute the bits on that, then zero-extend back to the original BitWidth.
> 
> What specific problem were you worried about here?
It just seems to be re-calculating a value that we already have.

================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:1831
                            unsigned Depth, const Query &Q) {
   KnownBits Known(getBitWidth(V->getType(), Q.DL));
   computeKnownBits(V, DemandedElts, Known, Depth, Q);
----------------
This appears to be where Known is calculated.

================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:1875

   assert((Ty->isIntOrIntVectorTy(BitWidth) || Ty->isPtrOrPtrVectorTy()) &&
          "Not integer or pointer type!");
----------------
And it's asserted that the size is correct here.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109883