Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFC][Clang] Move set functions out BranchProtectionInfo. #98451

Merged
merged 5 commits into from
Jul 12, 2024

Conversation

DanielKristofKiss
Copy link
Member

To reduce build times move them to TargetCodeGenInfo.
Refactor of #98329

Move the to TargetCodeGenInfo.
Refactor of llvm#98329
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:ARM backend:AArch64 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels Jul 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 11, 2024

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-backend-arm
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-backend-aarch64

Author: Daniel Kiss (DanielKristofKiss)

Changes

To reduce build times move them to TargetCodeGenInfo.
Refactor of #98329


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

5 Files Affected:

  • (modified) clang/include/clang/Basic/TargetInfo.h (-23)
  • (modified) clang/lib/CodeGen/TargetInfo.cpp (+23)
  • (modified) clang/lib/CodeGen/TargetInfo.h (+8-1)
  • (modified) clang/lib/CodeGen/Targets/AArch64.cpp (+1-1)
  • (modified) clang/lib/CodeGen/Targets/ARM.cpp (+2-2)
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index cf7628553647c..a58fb5f979272 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -32,9 +32,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Frontend/OpenMP/OMPGridValues.h"
-#include "llvm/IR/Attributes.h"
 #include "llvm/IR/DerivedTypes.h"
-#include "llvm/IR/Function.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/VersionTuple.h"
@@ -1410,7 +1408,6 @@ class TargetInfo : public TransferrableTargetInfo,
     bool BranchProtectionPAuthLR;
     bool GuardedControlStack;
 
-  protected:
     const char *getSignReturnAddrStr() const {
       switch (SignReturnAddr) {
       case LangOptions::SignReturnAddressScopeKind::None:
@@ -1433,7 +1430,6 @@ class TargetInfo : public TransferrableTargetInfo,
       llvm_unreachable("Unexpected SignReturnAddressKeyKind");
     }
 
-  public:
     BranchProtectionInfo()
         : SignReturnAddr(LangOptions::SignReturnAddressScopeKind::None),
           SignKey(LangOptions::SignReturnAddressKeyKind::AKey),
@@ -1454,25 +1450,6 @@ class TargetInfo : public TransferrableTargetInfo,
       BranchProtectionPAuthLR = LangOpts.BranchProtectionPAuthLR;
       GuardedControlStack = LangOpts.GuardedControlStack;
     }
-
-    void setFnAttributes(llvm::Function &F) {
-      llvm::AttrBuilder FuncAttrs(F.getContext());
-      setFnAttributes(FuncAttrs);
-      F.addFnAttrs(FuncAttrs);
-    }
-
-    void setFnAttributes(llvm::AttrBuilder &FuncAttrs) {
-      if (SignReturnAddr != LangOptions::SignReturnAddressScopeKind::None) {
-        FuncAttrs.addAttribute("sign-return-address", getSignReturnAddrStr());
-        FuncAttrs.addAttribute("sign-return-address-key", getSignKeyStr());
-      }
-      if (BranchTargetEnforcement)
-        FuncAttrs.addAttribute("branch-target-enforcement");
-      if (BranchProtectionPAuthLR)
-        FuncAttrs.addAttribute("branch-protection-pauth-lr");
-      if (GuardedControlStack)
-        FuncAttrs.addAttribute("guarded-control-stack");
-    }
   };
 
   /// Determine if the Architecture in this TargetInfo supports branch
diff --git a/clang/lib/CodeGen/TargetInfo.cpp b/clang/lib/CodeGen/TargetInfo.cpp
index 60224d458f6a2..9ccf0dea9a738 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -19,6 +19,7 @@
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/IR/Function.h"
 #include "llvm/IR/Type.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -206,6 +207,28 @@ llvm::Value *TargetCodeGenInfo::createEnqueuedBlockKernel(
   return F;
 }
 
+void TargetCodeGenInfo::setFnAttributes(
+    const TargetInfo::BranchProtectionInfo &BPI, llvm::Function &F) const {
+  llvm::AttrBuilder FuncAttrs(F.getContext());
+  setFnAttributes(BPI, FuncAttrs);
+  F.addFnAttrs(FuncAttrs);
+}
+
+void TargetCodeGenInfo::setFnAttributes(
+    const TargetInfo::BranchProtectionInfo &BPI,
+    llvm::AttrBuilder &FuncAttrs) const {
+  if (BPI.SignReturnAddr != LangOptions::SignReturnAddressScopeKind::None) {
+    FuncAttrs.addAttribute("sign-return-address", BPI.getSignReturnAddrStr());
+    FuncAttrs.addAttribute("sign-return-address-key", BPI.getSignKeyStr());
+  }
+  if (BPI.BranchTargetEnforcement)
+    FuncAttrs.addAttribute("branch-target-enforcement");
+  if (BPI.BranchProtectionPAuthLR)
+    FuncAttrs.addAttribute("branch-protection-pauth-lr");
+  if (BPI.GuardedControlStack)
+    FuncAttrs.addAttribute("guarded-control-stack");
+}
+
 namespace {
 class DefaultTargetCodeGenInfo : public TargetCodeGenInfo {
 public:
diff --git a/clang/lib/CodeGen/TargetInfo.h b/clang/lib/CodeGen/TargetInfo.h
index f242d9e36ed40..78c2f94508f8e 100644
--- a/clang/lib/CodeGen/TargetInfo.h
+++ b/clang/lib/CodeGen/TargetInfo.h
@@ -15,11 +15,12 @@
 #define LLVM_CLANG_LIB_CODEGEN_TARGETINFO_H
 
 #include "CGBuilder.h"
-#include "CodeGenModule.h"
 #include "CGValue.h"
+#include "CodeGenModule.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SyncScope.h"
+#include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -413,6 +414,12 @@ class TargetCodeGenInfo {
     return nullptr;
   }
 
+  void setFnAttributes(const TargetInfo::BranchProtectionInfo &BPI,
+                       llvm::Function &F) const;
+
+  void setFnAttributes(const TargetInfo::BranchProtectionInfo &BPI,
+                       llvm::AttrBuilder &FuncAttrs) const;
+
 protected:
   static std::string qualifyWindowsLibrary(StringRef Lib);
 
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp
index 3891f9fc8174b..d4b3c87705f08 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -133,7 +133,7 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo {
       }
     }
     auto *Fn = cast<llvm::Function>(GV);
-    BPI.setFnAttributes(*Fn);
+    CGM.getTargetCodeGenInfo().setFnAttributes(BPI, *Fn);
   }
 
   bool isScalarizableAsmOperand(CodeGen::CodeGenFunction &CGF,
diff --git a/clang/lib/CodeGen/Targets/ARM.cpp b/clang/lib/CodeGen/Targets/ARM.cpp
index 93fea94a77248..b2d4248b710c9 100644
--- a/clang/lib/CodeGen/Targets/ARM.cpp
+++ b/clang/lib/CodeGen/Targets/ARM.cpp
@@ -152,7 +152,7 @@ class ARMTargetCodeGenInfo : public TargetCodeGenInfo {
               diag::warn_target_unsupported_branch_protection_attribute)
               << Arch;
         } else {
-          BPI.setFnAttributes(*Fn);
+          CGM.getTargetCodeGenInfo().setFnAttributes(BPI, (*Fn));
         }
       } else if (CGM.getLangOpts().BranchTargetEnforcement ||
                  CGM.getLangOpts().hasSignReturnAddress()) {
@@ -168,7 +168,7 @@ class ARMTargetCodeGenInfo : public TargetCodeGenInfo {
     } else if (CGM.getTarget().isBranchProtectionSupportedArch(
                    CGM.getTarget().getTargetOpts().CPU)) {
       TargetInfo::BranchProtectionInfo BPI(CGM.getLangOpts());
-      BPI.setFnAttributes(*Fn);
+      CGM.getTargetCodeGenInfo().setFnAttributes(BPI, (*Fn));
     }
 
     const ARMInterruptAttr *Attr = FD->getAttr<ARMInterruptAttr>();

clang/lib/CodeGen/Targets/AArch64.cpp Outdated Show resolved Hide resolved
clang/lib/CodeGen/TargetInfo.h Outdated Show resolved Hide resolved
@nikic nikic requested a review from efriedma-quic July 11, 2024 13:12
Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one minor comment

clang/lib/CodeGen/Targets/ARM.cpp Show resolved Hide resolved
@DanielKristofKiss
Copy link
Member Author

#83277 needs the setBranchProtectionFnAttributes function but that call is from a static function so let's make it static.

Copy link

github-actions bot commented Jul 11, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@DanielKristofKiss DanielKristofKiss merged commit f34a165 into llvm:main Jul 12, 2024
7 checks passed
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
To reduce build times move them to TargetCodeGenInfo.

Refactor of llvm#98329
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 backend:ARM clang:codegen clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants