chill added a comment.
That said, my comments are not of the "over my dead body" kind ;)
================
Comment at: clang/lib/CodeGen/CGCall.cpp:1828
+ if (CodeGenOpts.BranchTargetEnforcement) {
+ FuncAttrs.addAttribute("branch-target-enforcement", "true");
+ }
----------------
I would really prefer to not set values "true" or "false" for the attribute: we
don't really have tri-state logic there (absent/present-true/present-false),
and those values just add some not-very useful string processing.
================
Comment at: clang/lib/CodeGen/CGCall.cpp:1831
+
+ auto RASignKind = CodeGenOpts.getSignReturnAddress();
+ if (RASignKind != CodeGenOptions::SignReturnAddressScope::None) {
----------------
What do we get from setting the PACBTI state in the default function
attributes? We still have
to do a per function processing, we can just as well avoid repeating the logic,
and spare us some
adding and potentially removing attributes churn.
================
Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:200
+ if (!F.hasFnAttribute("branch-target-enforcement"))
+ return false;
+ Attribute A = F.getFnAttribute("branch-target-enforcement");
----------------
This should be "true", although the comment might turn out moot.
If we somehow end up with a function, that does not have that attribute, we
should clear the
ELF flag.
================
Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:201-202
+ return false;
+ Attribute A = F.getFnAttribute("branch-target-enforcement");
+ return !A.isStringAttribute() || A.getValueAsString() == "false";
})) {
----------------
... that kind of string processing, here and elsewhere.
================
Comment at: llvm/lib/Target/AArch64/AArch64BranchTargets.cpp:62-66
+ Attribute A = F.getFnAttribute("branch-target-enforcement");
+ if (A.isStringAttribute() && A.getValueAsString() == "false")
+ return false;
+
+ if (!F.hasFnAttribute("branch-target-enforcement") &&
----------------
Isn't there some redundancy with the two calls to `getFnAttribute` and to
`hasFnAttribute` ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75181/new/
https://reviews.llvm.org/D75181
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
