[Builtins] Support ext_vector_type args for __builtin_fminf.

[fhahn]

Summary

At the moment, the various math builtins only accept the scalar types
defined by libm.

This patch extends __builtin_fminf to also accept ext_vector_type
arguments that can be converted to an ext_vector_type with float as
element type.

If this direction makes sense, I am planning to also convert a few other
builtins that can be extended to vectors directly, like fmax.

In addition to vector types, it would be also good to support matrix
types as follow up I think.

I tried to make sure the same error messages are emitted for the scalar
variant of __builtin_fminf, but there's a difference for the (vector,
float) variant. I still need to dig into where this is coming from.

[fhahn]

This brings Clang more in line with GCC, which already supports math builtins with ext_vector_type arguments.

This is unfortunately not correct. When I tried this with GCC the ext_vector_type attribute was dropped and it fell back to float.

[erichkeane]

I think I would want some level of examination/analysis as to whether we want to do this with all of the vector-types, instead of just the ext_vector_type.

clang/lib/Sema/SemaChecking.cpp

16544

Doing this as ExprResult(true) is absurdly jarring.

16554

I wonder if this should just deal in VectorType, which is the type that intends to emulate the GCC vector types. My understanding is ExtVectorType is for the clang-extended vector types.

16575

What does this comment come from? Is this supposed to be a 'FIXME'? I don't see any other matrix stuff here.

[fhahn]

Updated to remove stray comment.

In D108832#2969512, @erichkeane wrote:

I think I would want some level of examination/analysis as to whether we want to do this with all of the vector-types, instead of just the ext_vector_type.

I think we should extend this to all vector types and matrix types. I only went with ext_vector_types to keep things simple initially to make sure the direction is acceptable first.

[fhahn]

Use ExprError() instead of ExprResult(true).

[fhahn]

clang/lib/Sema/SemaChecking.cpp

16544

Updated, thanks!

16554

I think we should extend this to all vector types and matrix types. I only went with ext_vector_types to keep things simple initially to make sure the direction is acceptable first.

16575

Ah sorry, that was a left-over from where I get the boilerplate code to start with. It's removed in the latest version.

[erichkeane]

Codewise I'm ok, but I don't feel comfortable making the direction decision. I'd like to see some level of 'state of things' on the vector types (like, under what conditions does GCC support this, and under what types??), but even then I'm not sure I'm the right one to approve this.

[fhahn]

In D108832#2969538, @erichkeane wrote:

Codewise I'm ok, but I don't feel comfortable making the direction decision. I'd like to see some level of 'state of things' on the vector types (like, under what conditions does GCC support this, and under what types??), but even then I'm not sure I'm the right one to approve this.

Thanks! I'm preparing a proposal for cfe-dev at the moment. In addition to supporting element wise min/max/round & co I think we should also make sure the proposal also allows adding reduction versions in a consistent manner. I don't think that would be possible when overloading the existing builtins, so I am exploring a different direction at the moment.

Marking as changes planned for now.

[fhahn]

The vector builtin support took a different direction, see D111529 and following

clang/include/clang/Basic/Builtins.def

 BUILTIN(__builtin_fmal, "LdLdLdLd", "Fne")
 BUILTIN(__builtin_fmaf128, "LLdLLdLLdLLd", "Fne")
 BUILTIN(__builtin_fmax, "ddd", "Fnc")
 BUILTIN(__builtin_fmaxf, "fff", "Fnc")
 BUILTIN(__builtin_fmaxf16, "hhh", "Fnc")
 BUILTIN(__builtin_fmaxl, "LdLdLd", "Fnc")
 BUILTIN(__builtin_fmaxf128, "LLdLLdLLd", "Fnc")
 BUILTIN(__builtin_fmin, "ddd", "Fnc")
-BUILTIN(__builtin_fminf, "fff", "Fnc")
+BUILTIN(__builtin_fminf, "v.", "t")
 BUILTIN(__builtin_fminf16, "hhh", "Fnc")
 BUILTIN(__builtin_fminl, "LdLdLd", "Fnc")
 BUILTIN(__builtin_fminf128, "LLdLLdLLd", "Fnc")
 BUILTIN(__builtin_hypot , "ddd"  , "Fne")
 BUILTIN(__builtin_hypotf, "fff"  , "Fne")
 BUILTIN(__builtin_hypotl, "LdLdLd", "Fne")
 BUILTIN(__builtin_hypotf128, "LLdLLdLLd", "Fne")
 BUILTIN(__builtin_ilogb , "id", "Fne")

clang/include/clang/Sema/Sema.h

private:
   bool SemaBuiltinARMSpecialReg(unsigned BuiltinID, CallExpr *TheCall,
                                 int ArgNum, unsigned ExpectedFieldNum,
                                 bool AllowName);
   bool SemaBuiltinARMMemoryTaggingCall(unsigned BuiltinID, CallExpr *TheCall);
   bool SemaBuiltinPPCMMACall(CallExpr *TheCall, const char *TypeDesc);
 
   bool CheckPPCMMAType(QualType Type, SourceLocation TypeLoc);
 
+  ExprResult SemaBuiltinOverloadedMathBuiltin(CallExpr *TheCall,
+                                              ExprResult CallResult,
+                                              QualType ElementType);
+
   // Matrix builtin handling.
   ExprResult SemaBuiltinMatrixTranspose(CallExpr *TheCall,
                                         ExprResult CallResult);
   ExprResult SemaBuiltinMatrixColumnMajorLoad(CallExpr *TheCall,
                                               ExprResult CallResult);
   ExprResult SemaBuiltinMatrixColumnMajorStore(CallExpr *TheCall,
                                                ExprResult CallResult);
 

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_fminf:
+    return SemaBuiltinOverloadedMathBuiltin(TheCall, TheCallResult,
+                                            Context.FloatTy);
+
   case Builtin::BI__builtin_matrix_transpose:
     return SemaBuiltinMatrixTranspose(TheCall, TheCallResult);
 
   case Builtin::BI__builtin_matrix_column_major_load:
     return SemaBuiltinMatrixColumnMajorLoad(TheCall, TheCallResult);
 
   case Builtin::BI__builtin_matrix_column_major_store:
     return SemaBuiltinMatrixColumnMajorStore(TheCall, TheCallResult);

 void Sema::CheckAddressOfPackedMember(Expr *rhs) {
   using namespace std::placeholders;
 
   RefersToMemberWithReducedAlignment(
       rhs, std::bind(&Sema::AddPotentialMisalignedMembers, std::ref(*this), _1,
                      _2, _3, _4));
 }
 
+ExprResult Sema::SemaBuiltinOverloadedMathBuiltin(CallExpr *TheCall,
+                                                  ExprResult CallResult,
+                                                  QualType ElementType) {
+  if (checkArgCount(*this, TheCall, 2))
+    return ExprError();
+
+  // Apply default Lvalue conversions and convert the expression to size_t.
+  auto ApplyArgumentConversions = [this](Expr *E, QualType T) {
+    ExprResult Res(E);
+    AssignConvertType LHSConvTy = CheckSingleAssignmentConstraints(T, Res);
+    if (DiagnoseAssignmentResult(LHSConvTy, E->getBeginLoc(), T, E->getType(),
+                                 E, AA_Passing))
+      return ExprResult(true);
+
+    ExprResult Conv = DefaultLvalueConversion(E);
+    assert(
+        !Conv.isInvalid() &&
+        "conversion should be possible according to DiagnoseAssignmentResult");
+
+    return tryConvertExprToType(Conv.get(), T);
+  };
+
+  if (auto *VecType = ElementType->getAs<ExtVectorType>())
+    ElementType = VecType->getElementType();
+
+  QualType ArgTy = ElementType;
+  if (auto *VecType = TheCall->getArg(0)->getType()->getAs<ExtVectorType>()) {
+    ArgTy = Context.getExtVectorType(ElementType, VecType->getNumElements());
+  }
+
+  ExprResult A = ApplyArgumentConversions(TheCall->getArg(0), ArgTy);
+  if (A.isInvalid())
+    return A;
+
+  ExprResult B = ApplyArgumentConversions(TheCall->getArg(1), ArgTy);
+  if (B.isInvalid())
+    return B;
+
+  TheCall->setType(ArgTy);
+
+  auto *D = TheCall->getCalleeDecl();
+  D->addAttr(ConstAttr::CreateImplicit(Context, D->getLocation()));
+
+  // Update call argument to use the possibly converted matrix argument.
+  TheCall->setArg(0, A.get());
+  TheCall->setArg(1, B.get());
+  return CallResult;
+}
+
 ExprResult Sema::SemaBuiltinMatrixTranspose(CallExpr *TheCall,
                                             ExprResult CallResult) {
   if (checkArgCount(*this, TheCall, 1))
     return ExprError();
 
   ExprResult MatrixArg = DefaultLvalueConversion(TheCall->getArg(0));
   if (MatrixArg.isInvalid())
     return MatrixArg;

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

+// RUN: %clang_cc1 -triple=x86_64-apple-darwin -S -o - -emit-llvm %s | FileCheck %s
+
+typedef float float4 __attribute__((ext_vector_type(4)));
+
+void foo(float f, double d, float4 v) {
+  // CHECK:  call float @llvm.minnum.f32(float {{.*}}, float {{.*}})
+  f = __builtin_fminf(f, f);
+
+  // CHECK:  [[CONV1:%.*]] = fptrunc double {{.*}} to float
+  // CHECK:  [[CONV2:%.*]] = fptrunc double {{.*}} to float
+  // CHECK:  call float @llvm.minnum.f32(float [[CONV1]], float [[CONV2]])
+  f = __builtin_fminf(d, d);
+
+  // CHECK:   call <4 x float> @llvm.minnum.v4f32(<4 x float> {{.*}}, <4 x float> {{.*}})
+  v = __builtin_fminf(v, v);
+};

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

 // RUN: %clang_cc1 %s -pedantic -verify -triple=x86_64-apple-darwin9
 
 typedef float float4 __attribute__((ext_vector_type(4)));
 
 float test_fminf(float f, int i, int *ptr, float4 v) {
   float r1 = __builtin_fminf(f, ptr);
   // expected-error@-1 {{passing 'int *' to parameter of incompatible type 'float'}}
   float r2 = __builtin_fminf(ptr, f);
   // expected-error@-1 {{passing 'int *' to parameter of incompatible type 'float'}}
   float r3 = __builtin_fminf(v, f);
-  // expected-error@-1 {{passing 'float4' (vector of 4 'float' values) to parameter of incompatible type 'float'}}
+  // expected-error@-1 {{initializing 'float' with an expression of incompatible type 'float __attribute__((ext_vector_type(4)))' (vector of 4 'float' values)}}
   float r4 = __builtin_fminf(f, v);
   // expected-error@-1 {{passing 'float4' (vector of 4 'float' values) to parameter of incompatible type 'float'}}
 
 
   int *r5 = __builtin_fminf(f, f);
   // expected-error@-1 {{initializing 'int *' with an expression of incompatible type 'float'}}
 
   int *r6 = __builtin_fminf(f, v);
   // expected-error@-1 {{passing 'float4' (vector of 4 'float' values) to parameter of incompatible type 'float'}}
 }