[clang] [AArch64] Diagnose more functions when FP not enabled (PR #90832)

via cfe-commits cfe-commits at lists.llvm.org
Fri May 3 01:40:05 PDT 2024


https://github.com/ostannard updated https://github.com/llvm/llvm-project/pull/90832

>From b562b354a7bb955c3c46cd79753fd3d2d72d48fe Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Tue, 30 Apr 2024 11:40:18 +0100
Subject: [PATCH 1/3] [AArch64] Diagnose more functions when FP not enabled

When using a hard-float ABI for a target without FP registers, it's not
possible to correctly generate code for functions with arguments which
must be passed in floating-point registers. This is diagnosed in CodeGen
instead of Sema, to more closely match GCC's behaviour around inline
functions, which is relied on by the Linux kernel.

Previously, this only checked function signatures as they were
code-generated, but this missed some cases:
* Calls to functions not defined in this translation unit.
* Calls through function pointers.
* Calls to variadic functions, where the variadic arguments have a
  floating-point type.

This adds checks to function calls, as well as definitions, so that
these cases are correctly diagnosed.
---
 clang/lib/CodeGen/CGCall.cpp                  | 11 ++--
 clang/lib/CodeGen/TargetInfo.h                |  3 +-
 clang/lib/CodeGen/Targets/AArch64.cpp         | 66 ++++++++++++++++++-
 clang/lib/CodeGen/Targets/X86.cpp             | 16 +++--
 .../CodeGen/aarch64-soft-float-abi-errors.c   | 24 +++++++
 5 files changed, 106 insertions(+), 14 deletions(-)

diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index d2d92140b6b2a3..0c6234f92b8438 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5049,13 +5049,14 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
         (TargetDecl->hasAttr<TargetAttr>() ||
          (CurFuncDecl && CurFuncDecl->hasAttr<TargetAttr>())))
       checkTargetFeatures(Loc, FD);
-
-    // Some architectures (such as x86-64) have the ABI changed based on
-    // attribute-target/features. Give them a chance to diagnose.
-    CGM.getTargetCodeGenInfo().checkFunctionCallABI(
-        CGM, Loc, dyn_cast_or_null<FunctionDecl>(CurCodeDecl), FD, CallArgs);
   }
 
+  // Some architectures (such as x86-64) have the ABI changed based on
+  // attribute-target/features. Give them a chance to diagnose.
+  CGM.getTargetCodeGenInfo().checkFunctionCallABI(
+      CGM, Loc, dyn_cast_or_null<FunctionDecl>(CurCodeDecl),
+      dyn_cast_or_null<FunctionDecl>(TargetDecl), CallArgs, RetTy);
+
   // 1. Set up the arguments.
 
   // If we're using inalloca, insert the allocation after the stack save.
diff --git a/clang/lib/CodeGen/TargetInfo.h b/clang/lib/CodeGen/TargetInfo.h
index b1dfe5bf8f274d..f242d9e36ed40a 100644
--- a/clang/lib/CodeGen/TargetInfo.h
+++ b/clang/lib/CodeGen/TargetInfo.h
@@ -94,7 +94,8 @@ class TargetCodeGenInfo {
   virtual void checkFunctionCallABI(CodeGenModule &CGM, SourceLocation CallLoc,
                                     const FunctionDecl *Caller,
                                     const FunctionDecl *Callee,
-                                    const CallArgList &Args) const {}
+                                    const CallArgList &Args,
+                                    QualType ReturnType) const {}
 
   /// Determines the size of struct _Unwind_Exception on this platform,
   /// in 8-bit units.  The Itanium ABI defines this as:
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp
index 4c32f510101f04..4c66f7ffa04cf6 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -171,7 +171,22 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo {
   void checkFunctionCallABI(CodeGenModule &CGM, SourceLocation CallLoc,
                             const FunctionDecl *Caller,
                             const FunctionDecl *Callee,
-                            const CallArgList &Args) const override;
+                            const CallArgList &Args,
+                            QualType ReturnType) const override;
+
+private:
+  // Diagnose calls between functions with incompatible Streaming SVE
+  // attributes.
+  void checkFunctionCallABIStreaming(CodeGenModule &CGM, SourceLocation CallLoc,
+                                     const FunctionDecl *Caller,
+                                     const FunctionDecl *Callee) const;
+  // Diagnose calls which must pass arguments in floating-point registers when
+  // the selected target does not have floating-point registers.
+  void checkFunctionCallABISoftFloat(CodeGenModule &CGM, SourceLocation CallLoc,
+                                     const FunctionDecl *Caller,
+                                     const FunctionDecl *Callee,
+                                     const CallArgList &Args,
+                                     QualType ReturnType) const;
 };
 
 class WindowsAArch64TargetCodeGenInfo : public AArch64TargetCodeGenInfo {
@@ -881,9 +896,9 @@ void AArch64TargetCodeGenInfo::checkFunctionABI(
   }
 }
 
-void AArch64TargetCodeGenInfo::checkFunctionCallABI(
+void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
     CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
-    const FunctionDecl *Callee, const CallArgList &Args) const {
+    const FunctionDecl *Callee) const {
   if (!Caller || !Callee || !Callee->hasAttr<AlwaysInlineAttr>())
     return;
 
@@ -903,6 +918,51 @@ void AArch64TargetCodeGenInfo::checkFunctionCallABI(
           << Callee->getDeclName();
 }
 
+// If the target does not have floating-point registers, but we are using a
+// hard-float ABI, there is no way to pass floating-point, vector or HFA values
+// to functions, so we report an error.
+void AArch64TargetCodeGenInfo::checkFunctionCallABISoftFloat(
+    CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
+    const FunctionDecl *Callee, const CallArgList &Args,
+    QualType ReturnType) const {
+  const AArch64ABIInfo &ABIInfo = getABIInfo<AArch64ABIInfo>();
+  const TargetInfo &TI = ABIInfo.getContext().getTargetInfo();
+
+  if (!Caller || TI.hasFeature("fp") || ABIInfo.isSoftFloat())
+    return;
+
+  for (const CallArg &Arg : Args) {
+    const QualType Ty = Arg.getType();
+    const Type *HABase = nullptr;
+    uint64_t HAMembers = 0;
+
+    if (Ty->isFloatingType() || Ty->isVectorType() ||
+        ABIInfo.isHomogeneousAggregate(Ty, HABase, HAMembers)) {
+      CGM.getDiags().Report(CallLoc, diag::err_target_unsupported_type_for_abi)
+          << Caller->getDeclName() << Ty << TI.getABI();
+    }
+  }
+
+  const Type *HABase = nullptr;
+  uint64_t HAMembers = 0;
+
+  if (ReturnType->isFloatingType() || ReturnType->isVectorType() ||
+      ABIInfo.isHomogeneousAggregate(ReturnType, HABase, HAMembers)) {
+    CGM.getDiags().Report(CallLoc, diag::err_target_unsupported_type_for_abi)
+        << Caller->getDeclName() << ReturnType << TI.getABI();
+  }
+}
+
+void AArch64TargetCodeGenInfo::checkFunctionCallABI(CodeGenModule &CGM,
+                                                    SourceLocation CallLoc,
+                                                    const FunctionDecl *Caller,
+                                                    const FunctionDecl *Callee,
+                                                    const CallArgList &Args,
+                                                    QualType ReturnType) const {
+  checkFunctionCallABIStreaming(CGM, CallLoc, Caller, Callee);
+  checkFunctionCallABISoftFloat(CGM, CallLoc, Caller, Callee, Args, ReturnType);
+}
+
 void AArch64ABIInfo::appendAttributeMangling(TargetClonesAttr *Attr,
                                              unsigned Index,
                                              raw_ostream &Out) const {
diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp
index 94cf0d86f9bed7..717a27fc9c5744 100644
--- a/clang/lib/CodeGen/Targets/X86.cpp
+++ b/clang/lib/CodeGen/Targets/X86.cpp
@@ -1482,8 +1482,8 @@ class X86_64TargetCodeGenInfo : public TargetCodeGenInfo {
 
   void checkFunctionCallABI(CodeGenModule &CGM, SourceLocation CallLoc,
                             const FunctionDecl *Caller,
-                            const FunctionDecl *Callee,
-                            const CallArgList &Args) const override;
+                            const FunctionDecl *Callee, const CallArgList &Args,
+                            QualType ReturnType) const override;
 };
 } // namespace
 
@@ -1558,9 +1558,15 @@ static bool checkAVXParam(DiagnosticsEngine &Diag, ASTContext &Ctx,
   return false;
 }
 
-void X86_64TargetCodeGenInfo::checkFunctionCallABI(
-    CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
-    const FunctionDecl *Callee, const CallArgList &Args) const {
+void X86_64TargetCodeGenInfo::checkFunctionCallABI(CodeGenModule &CGM,
+                                                   SourceLocation CallLoc,
+                                                   const FunctionDecl *Caller,
+                                                   const FunctionDecl *Callee,
+                                                   const CallArgList &Args,
+                                                   QualType ReturnType) const {
+  if (!Callee)
+    return;
+
   llvm::StringMap<bool> CallerMap;
   llvm::StringMap<bool> CalleeMap;
   unsigned ArgIndex = 0;
diff --git a/clang/test/CodeGen/aarch64-soft-float-abi-errors.c b/clang/test/CodeGen/aarch64-soft-float-abi-errors.c
index 551e53bcd63d8a..6294fcf9a472f3 100644
--- a/clang/test/CodeGen/aarch64-soft-float-abi-errors.c
+++ b/clang/test/CodeGen/aarch64-soft-float-abi-errors.c
@@ -69,6 +69,7 @@ inline void test_float_arg_inline(float a) {}
 inline void test_float_arg_inline_used(float a) {}
 // nofp-hard-opt-error at -1 {{'a' requires 'float' type support, but ABI 'aapcs' does not support it}}
 void use_inline() { test_float_arg_inline_used(1.0f); }
+// nofp-hard-error at -1 {{'use_inline' requires 'float' type support, but ABI 'aapcs' does not support it}}
 
 // The always_inline attribute causes an inline function to always be
 // code-genned, even at -O0, so we always emit the error.
@@ -76,6 +77,7 @@ __attribute((always_inline))
 inline void test_float_arg_always_inline_used(float a) {}
 // nofp-hard-error at -1 {{'a' requires 'float' type support, but ABI 'aapcs' does not support it}}
 void use_always_inline() { test_float_arg_always_inline_used(1.0f); }
+// nofp-hard-error at -1 {{'use_always_inline' requires 'float' type support, but ABI 'aapcs' does not support it}}
 
 // Floating-point expressions, global variables and local variables do not
 // affect the ABI, so are allowed. GCC does reject some uses of floating point
@@ -97,3 +99,25 @@ int test_var_double(int a) {
   d *= 6.0;
   return (int)d;
 }
+
+extern void extern_float_arg(float);
+extern float extern_float_ret(void);
+void call_extern_float_arg() { extern_float_arg(1.0f); }
+// nofp-hard-error at -1 {{'call_extern_float_arg' requires 'float' type support, but ABI 'aapcs' does not support it}}
+void call_extern_float_ret() { extern_float_ret(); }
+// nofp-hard-error at -1 {{'call_extern_float_ret' requires 'float' type support, but ABI 'aapcs' does not support it}}
+
+// Definitions of variadic functions, and calls to them which only use integer
+// argument registers, are both fine.
+void variadic(int, ...);
+void call_variadic_int() { variadic(0, 1); }
+
+// Calls to variadic functions with floating-point arguments are an error,
+// since this would require floating-point registers.
+void call_variadic_double() { variadic(0, 1.0); }
+// nofp-hard-error at -1 {{'call_variadic_double' requires 'double' type support, but ABI 'aapcs' does not support it}}
+
+// Calls through function pointers are also diagnosed.
+void (*fptr)(float);
+void call_indirect() { fptr(1.0f); }
+// nofp-hard-error at -1 {{'call_indirect' requires 'float' type support, but ABI 'aapcs' does not support it}}

>From e27e14fc5102fec6ca04cc15e118a33087445f93 Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Thu, 2 May 2024 09:48:13 +0100
Subject: [PATCH 2/3] clang-format

---
 clang/lib/CodeGen/Targets/AArch64.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp
index 4c66f7ffa04cf6..1ba65ce4618f0b 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -170,8 +170,7 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo {
 
   void checkFunctionCallABI(CodeGenModule &CGM, SourceLocation CallLoc,
                             const FunctionDecl *Caller,
-                            const FunctionDecl *Callee,
-                            const CallArgList &Args,
+                            const FunctionDecl *Callee, const CallArgList &Args,
                             QualType ReturnType) const override;
 
 private:

>From c8ca58909f1d328b869da9b14033901bce8e687d Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Fri, 3 May 2024 09:36:26 +0100
Subject: [PATCH 3/3] Refactor duplicated code

---
 clang/lib/CodeGen/Targets/AArch64.cpp | 63 ++++++++++++---------------
 1 file changed, 27 insertions(+), 36 deletions(-)

diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp
index 1ba65ce4618f0b..452dc049d51b4f 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -867,30 +867,35 @@ static bool isStreamingCompatible(const FunctionDecl *F) {
   return false;
 }
 
+// Report an error if an argument or return value of type Ty would need to be
+// passed in a floating-point register.
+static void diagnoseIfNeedsFPReg(DiagnosticsEngine &Diags,
+                                 const StringRef ABIName,
+                                 const AArch64ABIInfo &ABIInfo,
+                                 const QualType &Ty, const NamedDecl *D) {
+  const Type *HABase = nullptr;
+  uint64_t HAMembers = 0;
+  if (Ty->isFloatingType() || Ty->isVectorType() ||
+      ABIInfo.isHomogeneousAggregate(Ty, HABase, HAMembers)) {
+    Diags.Report(D->getLocation(), diag::err_target_unsupported_type_for_abi)
+        << D->getDeclName() << Ty << ABIName;
+  }
+}
+
+// If we are using a hard-float ABI, but do not have floating point registers,
+// then report an error for any function arguments or returns which would be
+// passed in floating-pint registers.
 void AArch64TargetCodeGenInfo::checkFunctionABI(
     CodeGenModule &CGM, const FunctionDecl *FuncDecl) const {
   const AArch64ABIInfo &ABIInfo = getABIInfo<AArch64ABIInfo>();
   const TargetInfo &TI = ABIInfo.getContext().getTargetInfo();
 
-  // If we are using a hard-float ABI, but do not have floating point
-  // registers, then report an error for any function arguments or returns
-  // which would be passed in floating-pint registers.
-  auto CheckType = [&CGM, &TI, &ABIInfo](const QualType &Ty,
-                                         const NamedDecl *D) {
-    const Type *HABase = nullptr;
-    uint64_t HAMembers = 0;
-    if (Ty->isFloatingType() || Ty->isVectorType() ||
-        ABIInfo.isHomogeneousAggregate(Ty, HABase, HAMembers)) {
-      CGM.getDiags().Report(D->getLocation(),
-                            diag::err_target_unsupported_type_for_abi)
-          << D->getDeclName() << Ty << TI.getABI();
-    }
-  };
-
   if (!TI.hasFeature("fp") && !ABIInfo.isSoftFloat()) {
-    CheckType(FuncDecl->getReturnType(), FuncDecl);
+    diagnoseIfNeedsFPReg(CGM.getDiags(), TI.getABI(), ABIInfo,
+                         FuncDecl->getReturnType(), FuncDecl);
     for (ParmVarDecl *PVD : FuncDecl->parameters()) {
-      CheckType(PVD->getType(), PVD);
+      diagnoseIfNeedsFPReg(CGM.getDiags(), TI.getABI(), ABIInfo, PVD->getType(),
+                           PVD);
     }
   }
 }
@@ -930,26 +935,12 @@ void AArch64TargetCodeGenInfo::checkFunctionCallABISoftFloat(
   if (!Caller || TI.hasFeature("fp") || ABIInfo.isSoftFloat())
     return;
 
-  for (const CallArg &Arg : Args) {
-    const QualType Ty = Arg.getType();
-    const Type *HABase = nullptr;
-    uint64_t HAMembers = 0;
+  diagnoseIfNeedsFPReg(CGM.getDiags(), TI.getABI(), ABIInfo, ReturnType,
+                       Caller);
 
-    if (Ty->isFloatingType() || Ty->isVectorType() ||
-        ABIInfo.isHomogeneousAggregate(Ty, HABase, HAMembers)) {
-      CGM.getDiags().Report(CallLoc, diag::err_target_unsupported_type_for_abi)
-          << Caller->getDeclName() << Ty << TI.getABI();
-    }
-  }
-
-  const Type *HABase = nullptr;
-  uint64_t HAMembers = 0;
-
-  if (ReturnType->isFloatingType() || ReturnType->isVectorType() ||
-      ABIInfo.isHomogeneousAggregate(ReturnType, HABase, HAMembers)) {
-    CGM.getDiags().Report(CallLoc, diag::err_target_unsupported_type_for_abi)
-        << Caller->getDeclName() << ReturnType << TI.getABI();
-  }
+  for (const CallArg &Arg : Args)
+    diagnoseIfNeedsFPReg(CGM.getDiags(), TI.getABI(), ABIInfo, Arg.getType(),
+                         Caller);
 }
 
 void AArch64TargetCodeGenInfo::checkFunctionCallABI(CodeGenModule &CGM,



More information about the cfe-commits mailing list