[Clang] Add elementwise abs builtin.

[fhahn]

Summary

This patch implements __builtin_elementwise_abs as specified in
D111529.

[fhahn]

polish tests a bit

[craig.topper]

clang/lib/CodeGen/CGBuiltin.cpp

3109

Did we discuss that this is different than __builtin_abs which is undefined for INT_MIN?

[craig.topper]

clang/lib/CodeGen/CGBuiltin.cpp

3109

I don't think we add nsw to vector signed integer subtract, so I guess using false here is consistent.

[fhahn]

Pass is_int_min_poison=true to llvm.abs.* to reflect latest update to the specification to make taking abs of the most negative integer undefined.

[fhahn]

clang/lib/CodeGen/CGBuiltin.cpp

3109

Did we discuss that this is different than __builtin_abs which is undefined for INT_MIN?

So far it was not explicitly called out in the definition. I think we should explicitly make it undefined, so I updated the spec in D111529 and passed true here.

[scanon]

What's the rationale for making abs undefined on the minimum value? AFAIK every actual simd implementation defines the result and they agree (and even if one didn't, it would be pretty easy to get the "right" result. Introducing UB here just seems like punishing users for no reason.

[fhahn]

In D111986#3072946, @scanon wrote:

What's the rationale for making abs undefined on the minimum value? AFAIK every actual simd implementation defines the result and they agree (and even if one didn't, it would be pretty easy to get the "right" result. Introducing UB here just seems like punishing users for no reason.

Thanks! As making it UB does not really help with lowering I went back to allowing INT_MIN and updated the spec to explicitly spell out the behavior.

[scanon]

Two minor questions, but also LGTM as is.

clang/lib/Sema/SemaChecking.cpp

16538

Given that I expect this particular test to occur fairly frequently, maybe worth abstracting into some sort of get-elementwise-type operation.

clang/test/Sema/builtins-elementwise-math.c

25

For the purposes of C++ templates it might be nice to allow abs on unsigned (as the identity function). I don't have strong feelings though, and a library wrapping the builtins can do this themselves.

[fhahn]

Use checkMathBuiltinElementType helper added in D111985.

[fhahn]

clang/lib/Sema/SemaChecking.cpp

16538

moved out to a helper in the previous patch in the series, D111985

clang/test/Sema/builtins-elementwise-math.c

25

yeah that would be an option. I don't have any strong feelings either

[fhahn]

Rebased after recent changes in D111985: changes the return type of SemaBuiltinElementwiseMathOneArg to bool and drop the CallResult argument. Also added tests with const, typedef'ed and variables with address space

[aaron.ballman]

clang/include/clang/Basic/DiagnosticSemaKinds.td

8748

I feel like we must already have a diagnostic that covers this case...

clang/lib/Sema/SemaChecking.cpp

16548

Should this type undergo the usual promotions?

clang/test/Sema/builtins-elementwise-math.c

25

I was wondering about this myself, but also don't have strong opinions.

[fhahn]

Rebased and updated to use err_builtin_invalid_arg_type.

clang/include/clang/Basic/DiagnosticSemaKinds.td

8748

Updated to use err_builtin_invalid_arg_type.

clang/lib/Sema/SemaChecking.cpp

16548

I'm not sure, but given that we only have a single argument then wouldn't it be sufficient to avoid promotion? I don't think promotion to wider types would impact the results of the provided builtins.

[aaron.ballman]

clang/lib/Sema/SemaChecking.cpp

16548

You set the type of the call to be the type of the argument, which means passing in a const int will result in a const int that's observable and probably unexpected. e.g. this will fail,

const int a = -12;
static_assert(!std::is_const_v<decltype(__builtin_elementwise_abs(a))>);

(This can come up with overload resolution or in template specializations.)

[fhahn]

Apply UsualUnaryConversions to input argument.

[fhahn]

clang/lib/Sema/SemaChecking.cpp

16548

Ah right. I originally was planning on just using getUnqualifiedType, but for consistency it seems better to just apply the usual unary conversions. I also added the test above and a codegen tests with short.

[fhahn]

add comment to arg of Diag message

[aaron.ballman]

LGTM, thank you!

clang/include/clang/Basic/Builtins.def

 BUILTIN(__builtin_debugtrap, "v", "n")
 BUILTIN(__builtin_unreachable, "v", "nr")
 BUILTIN(__builtin_shufflevector, "v."   , "nct")
 BUILTIN(__builtin_convertvector, "v."   , "nct")
 BUILTIN(__builtin_alloca, "v*z"   , "Fn")
 BUILTIN(__builtin_alloca_with_align, "v*zIz", "Fn")
 BUILTIN(__builtin_call_with_static_chain, "v.", "nt")
 
+BUILTIN(__builtin_elementwise_abs, "v.", "nct")
 BUILTIN(__builtin_elementwise_max, "v.", "nct")
 BUILTIN(__builtin_elementwise_min, "v.", "nct")
 
 BUILTIN(__builtin_matrix_transpose, "v.", "nFt")
 BUILTIN(__builtin_matrix_column_major_load, "v.", "nFt")
 BUILTIN(__builtin_matrix_column_major_store, "v.", "nFt")
 
 // "Overloaded" Atomic operator builtins.  These are overloaded to support data

clang/include/clang/Basic/DiagnosticSemaKinds.td

def note_duplicate_asm_operand_name : Note<
 
   def error_inoutput_conflict_with_clobber : Error<
     "asm-specifier for input or output variable conflicts with asm"
     " clobber list">;
 
 let CategoryName = "Semantic Issue" in {
 
 def err_invalid_conversion_between_matrixes : Error<
   "conversion between matrix types%diff{ $ and $|}0,1 of different size is not allowed">;
 
 def err_invalid_conversion_between_matrix_and_type : Error<
   "conversion between matrix type %0 and incompatible type %1 is not allowed">;
 
 def err_invalid_conversion_between_vectors : Error<
   "invalid conversion between vector type%diff{ $ and $|}0,1 of different "
   "size">;
 def err_invalid_conversion_between_vector_and_integer : Error<

 
 def err_builtin_launder_invalid_arg : Error<
   "%select{non-pointer|function pointer|void pointer}0 argument to "
   "'__builtin_launder' is not allowed">;
 
 def err_builtin_invalid_arg_type: Error <
   "%ordinal0 argument must be a "
   "%select{vector, integer or floating point type|matrix|"
-   "pointer to a valid matrix element type}1 (was %2)">;
+   "pointer to a valid matrix element type|"
+   "signed integer or floating point type}1 (was %2)">;
 
 def err_builtin_matrix_disabled: Error<
   "matrix types extension is disabled. Pass -fenable-matrix to enable it">;
 def err_matrix_index_not_integer: Error<
   "matrix %select{row|column}0 index is not an integer">;
 def err_matrix_index_outside_range: Error<
   "matrix %select{row|column}0 index is outside the allowed range [0, %1)">;
 def err_matrix_incomplete_index: Error<

clang/include/clang/Sema/Sema.h

bool SemaBuiltinARMSpecialReg(unsigned BuiltinID, CallExpr *TheCall,
                                 bool AllowName);
   bool SemaBuiltinARMMemoryTaggingCall(unsigned BuiltinID, CallExpr *TheCall);
   bool SemaBuiltinPPCMMACall(CallExpr *TheCall, unsigned BuiltinID,
                              const char *TypeDesc);
 
   bool CheckPPCMMAType(QualType Type, SourceLocation TypeLoc);
 
   bool SemaBuiltinElementwiseMath(CallExpr *TheCall);
+  bool SemaBuiltinElementwiseMathOneArg(CallExpr *TheCall);
 
   // Matrix builtin handling.
   ExprResult SemaBuiltinMatrixTranspose(CallExpr *TheCall,
                                         ExprResult CallResult);
   ExprResult SemaBuiltinMatrixColumnMajorLoad(CallExpr *TheCall,
                                               ExprResult CallResult);
   ExprResult SemaBuiltinMatrixColumnMajorStore(CallExpr *TheCall,
                                                ExprResult CallResult);

clang/lib/CodeGen/CGBuiltin.cpp

Value *Sub =
         Builder.CreateSub(llvm::ConstantInt::get(IntTy, ExpMask), AbsV);
     // V = sign bit (Sub) <=> V = (Sub < 0)
     V = Builder.CreateLShr(Sub, llvm::ConstantInt::get(IntTy, bitsize - 1));
     if (bitsize > 32)
       V = Builder.CreateTrunc(V, ConvertType(E->getType()));
     return RValue::get(V);
   }
 
+  case Builtin::BI__builtin_elementwise_abs: {
+    Value *Op0 = EmitScalarExpr(E->getArg(0));
+    Value *Result;
+    if (Op0->getType()->isIntOrIntVectorTy())
+      Result = Builder.CreateBinaryIntrinsic(
+          llvm::Intrinsic::abs, Op0, Builder.getFalse(), nullptr, "elt.abs");
+    else
+      Result = Builder.CreateUnaryIntrinsic(llvm::Intrinsic::fabs, Op0, nullptr,
+                                            "elt.abs");
+    return RValue::get(Result);
+  }
   case Builtin::BI__builtin_elementwise_max: {
     Value *Op0 = EmitScalarExpr(E->getArg(0));
     Value *Op1 = EmitScalarExpr(E->getArg(1));
     Value *Result;
     if (Op0->getType()->isIntOrIntVectorTy()) {
       QualType Ty = E->getArg(0)->getType();
       if (auto *VecTy = Ty->getAs<VectorType>())
         Ty = VecTy->getElementType();

clang/lib/Sema/SemaChecking.cpp

if (!TheCall->getArg(0)->isValueDependent() &&
       Diag(TheCall->getBeginLoc(), diag::warn_frame_address)
           << ((BuiltinID == Builtin::BI__builtin_return_address)
                   ? "__builtin_return_address"
                   : "__builtin_frame_address")
           << TheCall->getSourceRange();
     break;
   }
 
+  case Builtin::BI__builtin_elementwise_abs:
+    if (SemaBuiltinElementwiseMathOneArg(TheCall))
+      return ExprError();
+    break;
   case Builtin::BI__builtin_elementwise_min:
   case Builtin::BI__builtin_elementwise_max:
     if (SemaBuiltinElementwiseMath(TheCall))
       return ExprError();
     break;
   case Builtin::BI__builtin_matrix_transpose:
     return SemaBuiltinMatrixTranspose(TheCall, TheCallResult);
 

 // Check if \p Ty is a valid type for the elementwise math builtins. If it is
 // not a valid type, emit an error message and return true. Otherwise return
 // false.
 static bool checkMathBuiltinElementType(Sema &S, SourceLocation Loc,
                                         QualType Ty) {
   if (!Ty->getAs<VectorType>() && !ConstantMatrixType::isValidElementType(Ty)) {
     S.Diag(Loc, diag::err_builtin_invalid_arg_type)
         << 1 << /* vector, integer or float ty*/ 0 << Ty;
     return true;
   }
   return false;
 }
 
+bool Sema::SemaBuiltinElementwiseMathOneArg(CallExpr *TheCall) {
+  if (checkArgCount(*this, TheCall, 1))
+    return true;
+
+  ExprResult A = UsualUnaryConversions(TheCall->getArg(0));
+  SourceLocation ArgLoc = TheCall->getArg(0)->getBeginLoc();
+  if (A.isInvalid())
+    return true;
+
+  TheCall->setArg(0, A.get());
+  QualType TyA = A.get()->getType();
+  if (checkMathBuiltinElementType(*this, ArgLoc, TyA))
+    return true;
+
+  QualType EltTy = TyA;
+  if (auto *VecTy = EltTy->getAs<VectorType>())
+    EltTy = VecTy->getElementType();
+  if (EltTy->isUnsignedIntegerType())
+    return Diag(ArgLoc, diag::err_builtin_invalid_arg_type)
+           << 1 << /*signed integer or float ty*/ 3 << TyA;
+
+  TheCall->setType(TyA);
+  return false;
+}
+
 bool Sema::SemaBuiltinElementwiseMath(CallExpr *TheCall) {
   if (checkArgCount(*this, TheCall, 2))
     return true;
 
   ExprResult A = TheCall->getArg(0);
   ExprResult B = TheCall->getArg(1);
   // Do standard promotions between the two arguments, returning their common
   // type.

clang/test/CodeGen/builtins-elementwise-math.c

 // RUN: %clang_cc1 -triple x86_64-apple-darwin %s -emit-llvm -disable-llvm-passes -o - | FileCheck %s
 
 typedef float float4 __attribute__((ext_vector_type(4)));
 typedef short int si8 __attribute__((ext_vector_type(8)));
 typedef unsigned int u4 __attribute__((ext_vector_type(4)));
 
 __attribute__((address_space(1))) int int_as_one;
 typedef int bar;
 bar b;
 
+void test_builtin_elementwise_abs(float f1, float f2, double d1, double d2,
+                                  float4 vf1, float4 vf2, si8 vi1, si8 vi2,
+                                  long long int i1, long long int i2, short si) {
+  // CHECK-LABEL: define void @test_builtin_elementwise_abs(
+  // CHECK:      [[F1:%.+]] = load float, float* %f1.addr, align 4
+  // CHECK-NEXT:  call float @llvm.fabs.f32(float [[F1]])
+  f2 = __builtin_elementwise_abs(f1);
+
+  // CHECK:      [[D1:%.+]] = load double, double* %d1.addr, align 8
+  // CHECK-NEXT: call double @llvm.fabs.f64(double [[D1]])
+  d2 = __builtin_elementwise_abs(d1);
+
+  // CHECK:      [[VF1:%.+]] = load <4 x float>, <4 x float>* %vf1.addr, align 16
+  // CHECK-NEXT: call <4 x float> @llvm.fabs.v4f32(<4 x float> [[VF1]])
+  vf2 = __builtin_elementwise_abs(vf1);
+
+  // CHECK:      [[I1:%.+]] = load i64, i64* %i1.addr, align 8
+  // CHECK-NEXT: call i64 @llvm.abs.i64(i64 [[I1]], i1 false)
+  i2 = __builtin_elementwise_abs(i1);
+
+  // CHECK:      [[VI1:%.+]] = load <8 x i16>, <8 x i16>* %vi1.addr, align 16
+  // CHECK-NEXT: call <8 x i16> @llvm.abs.v8i16(<8 x i16> [[VI1]], i1 false)
+  vi2 = __builtin_elementwise_abs(vi1);
+
+  // CHECK:      [[CVI2:%.+]] = load <8 x i16>, <8 x i16>* %cvi2, align 16
+  // CHECK-NEXT: call <8 x i16> @llvm.abs.v8i16(<8 x i16> [[CVI2]], i1 false)
+  const si8 cvi2 = vi2;
+  vi2 = __builtin_elementwise_abs(cvi2);
+
+  // CHECK:      [[IA1:%.+]] = load i32, i32 addrspace(1)* @int_as_one, align 4
+  // CHECK-NEXT: call i32 @llvm.abs.i32(i32 [[IA1]], i1 false)
+  b = __builtin_elementwise_abs(int_as_one);
+
+  // CHECK:   call i32 @llvm.abs.i32(i32 -10, i1 false)
+  b = __builtin_elementwise_abs(-10);
+
+  // CHECK:      [[SI:%.+]] = load i16, i16* %si.addr, align 2
+  // CHECK-NEXT: [[SI_EXT:%.+]] = sext i16 [[SI]] to i32
+  // CHECK-NEXT: [[RES:%.+]] = call i32 @llvm.abs.i32(i32 [[SI_EXT]], i1 false)
+  // CHECK-NEXT: = trunc i32 [[RES]] to i16
+  si = __builtin_elementwise_abs(si);
+}
+
 void test_builtin_elementwise_max(float f1, float f2, double d1, double d2,
                                   float4 vf1, float4 vf2, long long int i1,
                                   long long int i2, si8 vi1, si8 vi2,
                                   unsigned u1, unsigned u2, u4 vu1, u4 vu2) {
   // CHECK-LABEL: define void @test_builtin_elementwise_max(
-
   // CHECK:      [[F1:%.+]] = load float, float* %f1.addr, align 4
   // CHECK-NEXT: [[F2:%.+]] = load float, float* %f2.addr, align 4
   // CHECK-NEXT:  call float @llvm.maxnum.f32(float %0, float %1)
   f1 = __builtin_elementwise_max(f1, f2);
 
   // CHECK:      [[D1:%.+]] = load double, double* %d1.addr, align 8
   // CHECK-NEXT: [[D2:%.+]] = load double, double* %d2.addr, align 8
   // CHECK-NEXT: call double @llvm.maxnum.f64(double [[D1]], double [[D2]])

clang/test/Sema/builtins-elementwise-math.c

 // RUN: %clang_cc1 -std=c99 %s -pedantic -verify -triple=x86_64-apple-darwin9
 
 typedef float float4 __attribute__((ext_vector_type(4)));
 typedef int int3 __attribute__((ext_vector_type(3)));
+typedef unsigned unsigned4 __attribute__((ext_vector_type(4)));
 
 struct Foo {
   char *p;
 };
 
 __attribute__((address_space(1))) int int_as_one;
 typedef int bar;
 bar b;
 
+void test_builtin_elementwise_abs(int i, double d, float4 v, int3 iv, unsigned u, unsigned4 uv) {
+  struct Foo s = __builtin_elementwise_abs(i);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'int'}}
+
+  i = __builtin_elementwise_abs();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_elementwise_abs(i, i);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  i = __builtin_elementwise_abs(v);
+  // expected-error@-1 {{assigning to 'int' from incompatible type 'float4' (vector of 4 'float' values)}}
+
+  u = __builtin_elementwise_abs(u);
+  // expected-error@-1 {{1st argument must be a signed integer or floating point type (was 'unsigned int')}}
+
+  uv = __builtin_elementwise_abs(uv);
+  // expected-error@-1 {{1st argument must be a signed integer or floating point type (was 'unsigned4' (vector of 4 'unsigned int' values))}}
+}
+
 void test_builtin_elementwise_max(int i, short s, double d, float4 v, int3 iv, int *p) {
   i = __builtin_elementwise_max(p, d);
   // expected-error@-1 {{arguments are of different types ('int *' vs 'double')}}
 
   struct Foo foo = __builtin_elementwise_max(i, i);
   // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'int'}}
 
   i = __builtin_elementwise_max(i);

clang/test/SemaCXX/builtins-elementwise-math.cpp

 // RUN: %clang_cc1 %s -std=c++17 -pedantic -verify -triple=x86_64-apple-darwin9
 
 // Simple is_const implementation.
 struct true_type {
   static const bool value = true;
 };
 
 struct false_type {
   static const bool value = false;
 };
 
 template <class T> struct is_const : false_type {};
 template <class T> struct is_const<const T> : true_type {};
 
 // expected-no-diagnostics
 
+void test_builtin_elementwise_abs() {
+  const int a = 2;
+  int b = 1;
+  static_assert(!is_const<decltype(__builtin_elementwise_abs(a))>::value);
+  static_assert(!is_const<decltype(__builtin_elementwise_abs(b))>::value);
+}
+
 void test_builtin_elementwise_max() {
   const int a = 2;
   int b = 1;
   static_assert(!is_const<decltype(__builtin_elementwise_max(a, b))>::value);
   static_assert(!is_const<decltype(__builtin_elementwise_max(b, a))>::value);
   static_assert(!is_const<decltype(__builtin_elementwise_max(a, a))>::value);
 }
 
 void test_builtin_elementwise_min() {
   const int a = 2;
   int b = 1;
   static_assert(!is_const<decltype(__builtin_elementwise_min(a, b))>::value);
   static_assert(!is_const<decltype(__builtin_elementwise_min(b, a))>::value);
   static_assert(!is_const<decltype(__builtin_elementwise_min(a, a))>::value);
 }