aaron.ballman added inline comments.

================
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
----------------
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.."


================
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>())
----------------
Almost all of this logic is shared (except for picking the intrinsic), should 
it be combined?


================
Comment at: clang/test/CodeGen/builtins-elementwise-math.c:117
+  // CHECK-NEXT: call <4 x i32> @llvm.usub.sat.v4i32(<4 x i32> [[VU1]], <4 x 
i32> [[VU2]])
+  vu1 = __builtin_elementwise_sub_sat(vu1, vu2);
+
----------------
fhahn wrote:
> It might be good to have tests where one argument is signed and the other 
> unsigned as well. Those appear to be missing for other builtins as well 
> unfortunately.
I think it'd be good to have some codegen tests for `_BitInt` to make sure the 
behavior is correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117898

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to