[X86] Relax AVX ABI warning on internal functions

[jhuber6]

Summary:
The vector target should be able to handle vector sizes that are
multiples of the native size, this is useful for implementing math
routines that want to temporarily use a high precision for example.
However, currently this will emit a warning on x86 if any function calls
are involved. https://godbolt.org/z/dK7hGndYh.

I believe that we should be able to relax the ABI restriction if the
functions are completely internal and there were no explicitly states
attributes to conflict. Because the ABI is not exported outside the TU
we should be safe to assume that it won't bite us. In the case that one
call has no features and other does, that will still cause an error. I
may be wrong on this assumption however.

Fixes: #128361

[llvmbot]

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Joseph Huber (jhuber6)

Changes

Summary:
The vector target should be able to handle vector sizes that are
multiples of the native size, this is useful for implementing math
routines that want to temporarily use a high precision for example.
However, currently this will emit a warning on x86 if any function calls
are involved. https://godbolt.org/z/dK7hGndYh.

I believe that we should be able to relax the ABI restriction if the
functions are completely internal and there were no explicitly states
attributes to conflict. Because the ABI is not exported outside the TU
we should be safe to assume that it won't bite us. In the case that one
call has no features and other does, that will still cause an error. I
may be wrong on this assumption however.

---

Full diff: https://github.com/llvm/llvm-project/pull/157570.diff

2 Files Affected:

• (modified) clang/lib/CodeGen/Targets/X86.cpp (+13-7)
• (modified) clang/test/CodeGen/target-builtin-error-3.c (+11-1)

diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp
index 7ee8a0dc2a0db..c7be596a6d9a2 100644
--- a/clang/lib/CodeGen/Targets/X86.cpp
+++ b/clang/lib/CodeGen/Targets/X86.cpp
@@ -1513,12 +1513,18 @@ static void initFeatureMaps(const ASTContext &Ctx,
 
 static bool checkAVXParamFeature(DiagnosticsEngine &Diag,
                                  SourceLocation CallLoc,
+                                 const FunctionDecl &Callee,
                                  const llvm::StringMap<bool> &CallerMap,
                                  const llvm::StringMap<bool> &CalleeMap,
                                  QualType Ty, StringRef Feature,
                                  bool IsArgument) {
   bool CallerHasFeat = CallerMap.lookup(Feature);
   bool CalleeHasFeat = CalleeMap.lookup(Feature);
+  // No explicit features and the function is internal, be permissive.
+  if (!CallerHasFeat && !CalleeHasFeat &&
+      Callee.getFormalLinkage() == Linkage::Internal)
+    return false;
+
   if (!CallerHasFeat && !CalleeHasFeat)
     return Diag.Report(CallLoc, diag::warn_avx_calling_convention)
            << IsArgument << Ty << Feature;
@@ -1534,18 +1540,18 @@ static bool checkAVXParamFeature(DiagnosticsEngine &Diag,
 }
 
 static bool checkAVXParam(DiagnosticsEngine &Diag, ASTContext &Ctx,
-                          SourceLocation CallLoc,
+                          SourceLocation CallLoc, const FunctionDecl &Callee,
                           const llvm::StringMap<bool> &CallerMap,
                           const llvm::StringMap<bool> &CalleeMap, QualType Ty,
                           bool IsArgument) {
   uint64_t Size = Ctx.getTypeSize(Ty);
   if (Size > 256)
-    return checkAVXParamFeature(Diag, CallLoc, CallerMap, CalleeMap, Ty,
+    return checkAVXParamFeature(Diag, CallLoc, Callee, CallerMap, CalleeMap, Ty,
                                 "avx512f", IsArgument);
 
   if (Size > 128)
-    return checkAVXParamFeature(Diag, CallLoc, CallerMap, CalleeMap, Ty, "avx",
-                                IsArgument);
+    return checkAVXParamFeature(Diag, CallLoc, Callee, CallerMap, CalleeMap, Ty,
+                                "avx", IsArgument);
 
   return false;
 }
@@ -1582,8 +1588,8 @@ void X86_64TargetCodeGenInfo::checkFunctionCallABI(CodeGenModule &CGM,
       if (ArgIndex < Callee->getNumParams())
         Ty = Callee->getParamDecl(ArgIndex)->getType();
 
-      if (checkAVXParam(CGM.getDiags(), CGM.getContext(), CallLoc, CallerMap,
-                        CalleeMap, Ty, /*IsArgument*/ true))
+      if (checkAVXParam(CGM.getDiags(), CGM.getContext(), CallLoc, *Callee,
+                        CallerMap, CalleeMap, Ty, /*IsArgument*/ true))
         return;
     }
     ++ArgIndex;
@@ -1594,7 +1600,7 @@ void X86_64TargetCodeGenInfo::checkFunctionCallABI(CodeGenModule &CGM,
   if (Callee->getReturnType()->isVectorType() &&
       CGM.getContext().getTypeSize(Callee->getReturnType()) > 128) {
     initFeatureMaps(CGM.getContext(), CallerMap, Caller, CalleeMap, Callee);
-    checkAVXParam(CGM.getDiags(), CGM.getContext(), CallLoc, CallerMap,
+    checkAVXParam(CGM.getDiags(), CGM.getContext(), CallLoc, *Callee, CallerMap,
                   CalleeMap, Callee->getReturnType(),
                   /*IsArgument*/ false);
   }
diff --git a/clang/test/CodeGen/target-builtin-error-3.c b/clang/test/CodeGen/target-builtin-error-3.c
index 3de76e253d9b1..77bb16b49a741 100644
--- a/clang/test/CodeGen/target-builtin-error-3.c
+++ b/clang/test/CodeGen/target-builtin-error-3.c
@@ -24,6 +24,16 @@ static inline half16 __attribute__((__overloadable__)) convert_half( float16 a )
 }
 void avx_test( uint16_t *destData, float16 argbF)
 {
-  // expected-warning@+1{{AVX vector argument of type 'float16' (vector of 16 'float' values) without 'avx512f' enabled changes the ABI}}
   ((half16U *)destData)[0] = convert_half(argbF);
 }
+
+half16 test( float16 a ) {
+  half16 r;
+  r.lo = convert_half(a.lo);
+  return r;
+}
+void avx_test2( uint16_t *destData, float16 argbF)
+{
+  // expected-warning@+1{{AVX vector argument of type 'float16' (vector of 16 'float' values) without 'avx512f' enabled changes the ABI}}
+  ((half16U *)destData)[0] = test(argbF);
+}

[efriedma-quic]

The basic idea here is that if the caller and callee are defined in the same module, they must have the same command-line flags, and therefore the calling convention can't mismatch. That makes sense.

[rjmccall]

LGTM

[philnik777]

Does this actually fix the linked bug report? That's about always_inline functions, which still have default visibility.

[jhuber6]

Does this actually fix the linked bug report? That's about always_inline functions, which still have default visibility.

Right, forgot to do a check on that. Hopefully I can just check the attribute there.