[PATCH] D83964: GlobalISel: Augment addsat/subsat lowering with an optional type

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

Looks OK technically. As an alternative, would it be possible to widen, and then have the lowering spot that the operands are both extended from a smaller type, so they're known to have a small range, so we can use the more efficient lowering? That would avoid the need for this new "optional type" concept.



================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:5344-5345
+      unsigned NewNumBits = Ty.getScalarSizeInBits();
+      APInt MaxVal = APInt::getAllOnesValue(OrigNumBits);
+      auto SatMax = MIRBuilder.buildConstant(Ty, MaxVal.zext(NewNumBits));
+      auto Add = MIRBuilder.buildAdd(Ty, LHS, RHS);
----------------
Use APInt::getLowBitsSet?


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:5368-5373
     auto MaxVal =
-        MIRBuilder.buildConstant(Ty, APInt::getSignedMaxValue(NumBits));
+      MIRBuilder.buildConstant(
+        Ty, APInt::getSignedMaxValue(OrigNumBits).sextOrSelf(NewNumBits));
     auto MinVal =
-        MIRBuilder.buildConstant(Ty, APInt::getSignedMinValue(NumBits));
+      MIRBuilder.buildConstant(
+        Ty, APInt::getSignedMinValue(OrigNumBits).sextOrSelf(NewNumBits));
----------------
Could use getLowBitsSet and getOneBitSet, but I suppose it would be annoyingly asymmetrical.


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

https://reviews.llvm.org/D83964