-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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 functions of BranchProtectionInfo out of line #98329
Conversation
Also let's add const to the setFnAttributes.
@llvm/pr-subscribers-clang Author: Daniel Kiss (DanielKristofKiss) ChangesAlso let's add const to the setFnAttributes. Full diff: https://github.com/llvm/llvm-project/pull/98329.diff 2 Files Affected:
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 1f208b40f92cb..5a6a0cdbd316a 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -34,7 +34,6 @@
#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"
@@ -1450,24 +1449,9 @@ class TargetInfo : public TransferrableTargetInfo,
GuardedControlStack = LangOpts.GuardedControlStack;
}
- void setFnAttributes(llvm::Function &F) {
- llvm::AttrBuilder FuncAttrs(F.getContext());
- setFnAttributes(FuncAttrs);
- F.addFnAttrs(FuncAttrs);
- }
+ void setFnAttributes(llvm::Function &F) const;
- 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");
- }
+ void setFnAttributes(llvm::AttrBuilder &FuncAttrs) const;
};
/// Determine if the Architecture in this TargetInfo supports branch
diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index 29f5cd14e46e1..a99ddd081ae28 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -18,6 +18,7 @@
#include "clang/Basic/LangOptions.h"
#include "llvm/ADT/APFloat.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/IR/Function.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/TargetParser/TargetParser.h"
#include <cstdlib>
@@ -1005,3 +1006,22 @@ void TargetInfo::copyAuxTarget(const TargetInfo *Aux) {
auto *Src = static_cast<const TransferrableTargetInfo*>(Aux);
*Target = *Src;
}
+
+void TargetInfo::BranchProtectionInfo::setFnAttributes(llvm::Function &F) const {
+ llvm::AttrBuilder FuncAttrs(F.getContext());
+ setFnAttributes(FuncAttrs);
+ F.addFnAttrs(FuncAttrs);
+}
+
+void TargetInfo::BranchProtectionInfo::setFnAttributes(llvm::AttrBuilder &FuncAttrs) const {
+ 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");
+}
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
@@ -34,7 +34,6 @@ | |||
#include "llvm/Frontend/OpenMP/OMPGridValues.h" | |||
#include "llvm/IR/Attributes.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also drop the Attributes.h include.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/89/builds/1820 Here is the relevant piece of the build log for the reference:
|
in some build config |
Okay, that means that putting this code inside Basic/ is a layering violation. You need to move it into CodeGen/. Probably best to revert the whole change in the meantime. |
Already reverted, I'm on it. |
Move the to TargetCodeGenInfo. Refactor of llvm#98329
To reduce build times move them to TargetCodeGenInfo. Refactor of #98329
…#98329) Also let's add const to the setFnAttributes.
To reduce build times move them to TargetCodeGenInfo. Refactor of llvm#98329
Also let's add const to the setFnAttributes.