[PATCH] D102969: [GlobalISel] Add G_SBFX/G_UBFX to computeKnownBits

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp:510-511
+    Known = KnownBits::lshr(SrcOpKnown, OffsetKnown) &
+            KnownBits::computeForAddSub(/*Add*/ false, /*NSW*/ false,
+                                        KnownBits::shl(One, WidthKnown), One);
+    if (Opcode == TargetOpcode::G_SBFX) {
----------------
bcahoon wrote:
> foad wrote:
> > For example, instead of expanding the calculation "(1 << Width) - 1" literally here, I think you might get better results with something like:
> > ```
> >   KnownBits Mask;
> >   Mask.Zero = APInt::getBitsSetFrom(BitWidth, WidthKnown.getMaxValue());
> >   Mask.One = APInt::getLowBitsSet(BitWidth, WidthKnown.getMinValue());
> > ```
> Thanks for the suggestion. I use getBitsSetFrom/getLowBitsSet when the Width value is known.  I think generating the mask explicitly or creating the literal calculation should generate the same results. But, when the width is known, it makes sense to just create the mask explicitly.   
I don't understand why WidthKnown.isUnknown() should be a special case. But you might need some strategic `std::min`s to avoid passing a value larger than BitWidth into getBitsSetFrom/getLowBitsSet.


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

https://reviews.llvm.org/D102969