[PATCH] D117898: [Clang] Add elementwise saturated add/sub builtins

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

Aside from some nits, this LGTM.



================
Comment at: clang/docs/LanguageExtensions.rst:549
+ T __builtin_elementwise_add_sat(T x, T y) return the sum of x and y, clamped to the range of signed or     integer types
+                                           values representable by the bit width of the arguments.
+ T __builtin_elementwise_sub_sat(T x, T y) return the difference of x and y, clamped to the range of        integer types
----------------
aaron.ballman wrote:
> craig.topper wrote:
> > Not sure if I'm reading this right due to the columns, but is "unsigned" missing after the "signed or"
> This reads strangely to me as well. "..., clamped to the range of signed or integer types unsigned values representable by.."
This still seems unaddressed.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3157-3184
+  case Builtin::BI__builtin_elementwise_add_sat: {
+    Value *Op0 = EmitScalarExpr(E->getArg(0));
+    Value *Op1 = EmitScalarExpr(E->getArg(1));
+    Value *Result;
+    assert(Op0->getType()->isIntOrIntVectorTy() && "integer type expected");
+    QualType Ty = E->getArg(0)->getType();
+    if (auto *VecTy = Ty->getAs<VectorType>())
----------------
aaron.ballman wrote:
> Almost all of this logic is shared (except for picking the intrinsic), should it be combined?
Same with this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117898