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

[Leonard Chan via Phabricator via llvm-commits]

leonardchan added inline comments.


================
Comment at: lib/CodeGen/TargetLoweringBase.cpp:1883
+  //   SatMin -> (LHS < 0) && (RHS < 0) && (Sum > 0)
+  //
+  SDValue Zero = DAG.getConstant(0, dl, LHS.getValueType());
----------------
ebevhan wrote:
> Would there be a benefit to using SADDO here if it's legal? Minimum saturation occurs if `Overflow && Sum > 0` and maximum saturation if `Overflow && Sum < 0`.
> 
> It really depends on whether or not the target can chain the operations effectively, but I suspect that most targets won't be able to keep the overflow flag alive without materializing it.
I'm running into legality issues when type expansion is involved. I was able to replace this block to use SADDO but ran into an error saying the SADDO node was not expanded when legalizing types. I'm not sure if I should call another function to "force check" expansion on my newly created SADDO node, but I couldn't find any other examples that show usage of SADDO during expansion.

Alternatively, I could move the code for expanding SADDO into its own method that I could call from here. 


Repository:
  rL LLVM

https://reviews.llvm.org/D53053