[PATCH] D53053: [Intrinsic] Signed Saturation Addition Intrinsic

[Bevin Hansson via Phabricator via llvm-commits]

ebevhan added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:540
+SDValue DAGTypeLegalizer::PromoteIntRes_SATSADD(SDNode *N) {
+  SDValue LHS = SExtPromotedInteger(N->getOperand(0));
+  SDValue RHS = SExtPromotedInteger(N->getOperand(1));
----------------
craig.topper wrote:
> ebevhan wrote:
> > craig.topper wrote:
> > > This doesn't seem right. Let's say the input was an i8 SATSADD that should have returned [-128,127] with no wrap around. Let's say the type to promote to i16.  This becomes two sign_extend_inreg from i8 to i16 and an an i16 SATSADD, but that will only saturate when the math overflows 16 bits. So it's not longer saturating to [-128,127]. You get can [-256,254] now.
> > I think a more correct result could be obtained for iN to iM by doing ANY_EXTEND to iM->LSHR by M-N->SATSADD->ASHR M-N. I'm not sure if this is the best way, though.
> Should LSHR be SHL?
Oh, whoops. Yes, it should be SHL.


Repository:
  rL LLVM

https://reviews.llvm.org/D53053