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

[Leonard Chan via Phabricator via llvm-commits]

leonardchan added a comment.

In https://reviews.llvm.org/D53053#1263049, @craig.topper wrote:

> Are we not supporting vectors at all? That needs to be checked in the IR verifier if so. Though I would really like to see this for vectors since X86 has 8 and 16 bit saturating vector addition.


We can add them. Not sure if what I added is the correct way they should be expanded though. I might just not be looking hard enough, but I couldn't find other examples of intrinsics that take int vector arguments and expand them.



================
Comment at: lib/CodeGen/TargetLoweringBase.cpp:1890
+
+  SDValue SumPos = DAG.getSetCC(dl, BoolVT, Sum, Zero, ISD::SETGT);
+  SDValue SumNeg = DAG.getSetCC(dl, BoolVT, Sum, Zero, ISD::SETLT);
----------------
bjope wrote:
> Maybe I miss something, but shouldn't it be SETGE here?
> Assume that we have and SADDSAT(SignedMin, SignedMin). Then we overflow, and the result it zero. But we should still saturate to SignedMin.
You're right. It should be.


Repository:
  rL LLVM

https://reviews.llvm.org/D53053