[llvm][AArch64] Autoupgrade function attributes from Module attributes. #80640
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-clang-codegen Author: Dani (DanielKristofKiss) Changes
Full diff: https://github.com/llvm/llvm-project/pull/80640.diff 9 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index c63e4ecc3dcba..36b63d78b06f8 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1044,17 +1044,14 @@ void CodeGenModule::Release() {
llvm::MDString::get(VMContext, "ascii"));
}
- llvm::Triple::ArchType Arch = Context.getTargetInfo().getTriple().getArch();
- if ( Arch == llvm::Triple::arm
- || Arch == llvm::Triple::armeb
- || Arch == llvm::Triple::thumb
- || Arch == llvm::Triple::thumbeb) {
+ llvm::Triple T = Context.getTargetInfo().getTriple();
+ if (T.isARM() || T.isThumb()) {
// The minimum width of an enum in bytes
uint64_t EnumWidth = Context.getLangOpts().ShortEnums ? 1 : 4;
getModule().addModuleFlag(llvm::Module::Error, "min_enum_size", EnumWidth);
}
- if (Arch == llvm::Triple::riscv32 || Arch == llvm::Triple::riscv64) {
+ if (T.isRISCV()) {
StringRef ABIStr = Target.getABI();
llvm::LLVMContext &Ctx = TheModule.getContext();
getModule().addModuleFlag(llvm::Module::Error, "target-abi",
@@ -1127,10 +1124,7 @@ void CodeGenModule::Release() {
getModule().addModuleFlag(llvm::Module::Override,
"tag-stack-memory-buildattr", 1);
- if (Arch == llvm::Triple::thumb || Arch == llvm::Triple::thumbeb ||
- Arch == llvm::Triple::arm || Arch == llvm::Triple::armeb ||
- Arch == llvm::Triple::aarch64 || Arch == llvm::Triple::aarch64_32 ||
- Arch == llvm::Triple::aarch64_be) {
+ if (T.isARM() || T.isThumb() || T.isAArch64()) {
if (LangOpts.BranchTargetEnforcement)
getModule().addModuleFlag(llvm::Module::Min, "branch-target-enforcement",
1);
diff --git a/llvm/include/llvm/IR/AutoUpgrade.h b/llvm/include/llvm/IR/AutoUpgrade.h
index 152f781ffa9b3..c0d96efc54752 100644
--- a/llvm/include/llvm/IR/AutoUpgrade.h
+++ b/llvm/include/llvm/IR/AutoUpgrade.h
@@ -67,7 +67,8 @@ namespace llvm {
void UpgradeSectionAttributes(Module &M);
/// Correct any IR that is relying on old function attribute behavior.
- void UpgradeFunctionAttributes(Function &F);
+ void UpgradeFunctionAttributes(Function &F,
+ bool ModuleMetadataIsMaterialized = false);
/// If the given TBAA tag uses the scalar TBAA format, create a new node
/// corresponding to the upgrade to the struct-path aware TBAA format.
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 5b233fb365fe2..6b335dd9f1f89 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -6708,7 +6708,7 @@ Error BitcodeReader::materialize(GlobalValue *GV) {
}
// Look for functions that rely on old function attribute behavior.
- UpgradeFunctionAttributes(*F);
+ UpgradeFunctionAttributes(*F, true);
// If we've materialized a function set up in "new" debug-info mode, the
// contents just loaded will still be in dbg.value mode. Switch to the new
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 19d80eb9aec0b..e25ac46450cec 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -5155,7 +5155,39 @@ struct StrictFPUpgradeVisitor : public InstVisitor<StrictFPUpgradeVisitor> {
};
} // namespace
-void llvm::UpgradeFunctionAttributes(Function &F) {
+static void
+CopyModuleAttributeToFunction(Function &F, StringRef FnAttrName,
+ StringRef ModAttrName,
+ std::pair<StringRef, StringRef> Values) {
+ Module *M = F.getParent();
+ if (!M)
+ return;
+ if (F.hasFnAttribute(FnAttrName))
+ return;
+ if (const auto *MAttr = mdconst::extract_or_null<ConstantInt>(
+ M->getModuleFlag(ModAttrName))) {
+ if (MAttr->getZExtValue()) {
+ F.addFnAttr(FnAttrName, Values.first);
+ return;
+ }
+ }
+ F.addFnAttr(FnAttrName, Values.second);
+}
+
+static void CopyModuleAttributeToFunction(Function &F, StringRef AttrName) {
+ CopyModuleAttributeToFunction(
+ F, AttrName, AttrName,
+ std::make_pair<StringRef, StringRef>("true", "false"));
+}
+
+static void
+CopyModuleAttributeToFunction(Function &F, StringRef AttrName,
+ std::pair<StringRef, StringRef> Values) {
+ CopyModuleAttributeToFunction(F, AttrName, AttrName, Values);
+}
+
+void llvm::UpgradeFunctionAttributes(Function &F,
+ bool ModuleMetadataIsMaterialized) {
// If a function definition doesn't have the strictfp attribute,
// convert any callsite strictfp attributes to nobuiltin.
if (!F.isDeclaration() && !F.hasFnAttribute(Attribute::StrictFP)) {
@@ -5167,6 +5199,41 @@ void llvm::UpgradeFunctionAttributes(Function &F) {
F.removeRetAttrs(AttributeFuncs::typeIncompatible(F.getReturnType()));
for (auto &Arg : F.args())
Arg.removeAttrs(AttributeFuncs::typeIncompatible(Arg.getType()));
+
+ if (!ModuleMetadataIsMaterialized)
+ return;
+ if (F.isDeclaration())
+ return;
+ Module *M = F.getParent();
+ if (!M)
+ return;
+
+ Triple T(M->getTargetTriple());
+ // Convert module level attributes to function level attributes because
+ // after merging modules the attributes might change and would have different
+ // effect on the functions as the original module would have.
+ if (T.isThumb() || T.isARM() || T.isAArch64()) {
+ if (!F.hasFnAttribute("sign-return-address")) {
+ StringRef SignType = "none";
+ if (const auto *Sign = mdconst::extract_or_null<ConstantInt>(
+ M->getModuleFlag("sign-return-address")))
+ if (Sign->getZExtValue())
+ SignType = "non-leaf";
+
+ if (const auto *SignAll = mdconst::extract_or_null<ConstantInt>(
+ M->getModuleFlag("sign-return-address-all")))
+ if (SignAll->getZExtValue())
+ SignType = "all";
+
+ F.addFnAttr("sign-return-address", SignType);
+ }
+ CopyModuleAttributeToFunction(F, "branch-target-enforcement");
+ CopyModuleAttributeToFunction(F, "branch-protection-pauth-lr");
+ CopyModuleAttributeToFunction(F, "guarded-control-stack");
+ CopyModuleAttributeToFunction(
+ F, "sign-return-address-key",
+ std::make_pair<StringRef, StringRef>("b_key", "a_key"));
+ }
}
static bool isOldLoopArgument(Metadata *MD) {
diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index 8cc0f7fb90991..47d5a39c9f746 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -1606,6 +1606,10 @@ Error IRLinker::run() {
// Loop over all of the linked values to compute type mappings.
computeTypeMapping();
+ // Update function attributes before copy them to destation module.
+ for (Function &F : SrcM->getFunctionList())
+ UpgradeFunctionAttributes(F, true);
+
std::reverse(Worklist.begin(), Worklist.end());
while (!Worklist.empty()) {
GlobalValue *GV = Worklist.back();
diff --git a/llvm/test/Bitcode/upgrade-arc-runtime-calls.ll b/llvm/test/Bitcode/upgrade-arc-runtime-calls.ll
index 19f25f98953fa..d2edec18d55e5 100644
--- a/llvm/test/Bitcode/upgrade-arc-runtime-calls.ll
+++ b/llvm/test/Bitcode/upgrade-arc-runtime-calls.ll
@@ -55,7 +55,7 @@ unwindBlock:
// Check that auto-upgrader converts function calls to intrinsic calls. Note that
// the auto-upgrader doesn't touch invoke instructions.
-// ARC: define void @testRuntimeCalls(ptr %[[A:.*]], ptr %[[B:.*]], ptr %[[C:.*]], ptr %[[D:.*]], ptr %[[E:.*]]) personality
+// ARC: define void @testRuntimeCalls(ptr %[[A:.*]], ptr %[[B:.*]], ptr %[[C:.*]], ptr %[[D:.*]], ptr %[[E:.*]]) #0 personality
// ARC: %[[V0:.*]] = tail call ptr @llvm.objc.autorelease(ptr %[[A]])
// ARC-NEXT: tail call void @llvm.objc.autoreleasePoolPop(ptr %[[A]])
// ARC-NEXT: %[[V1:.*]] = tail call ptr @llvm.objc.autoreleasePoolPush()
@@ -88,7 +88,7 @@ unwindBlock:
// ARC-NEXT: tail call void @llvm.objc.arc.annotation.bottomup.bbend(ptr %[[B]], ptr %[[C]])
// ARC-NEXT: invoke void @objc_autoreleasePoolPop(ptr %[[A]])
-// NOUPGRADE: define void @testRuntimeCalls(ptr %[[A:.*]], ptr %[[B:.*]], ptr %[[C:.*]], ptr %[[D:.*]], ptr %[[E:.*]]) personality
+// NOUPGRADE: define void @testRuntimeCalls(ptr %[[A:.*]], ptr %[[B:.*]], ptr %[[C:.*]], ptr %[[D:.*]], ptr %[[E:.*]]) #0 personality
// NOUPGRADE: %[[V0:.*]] = tail call ptr @objc_autorelease(ptr %[[A]])
// NOUPGRADE-NEXT: tail call void @objc_autoreleasePoolPop(ptr %[[A]])
// NOUPGRADE-NEXT: %[[V1:.*]] = tail call ptr @objc_autoreleasePoolPush()
diff --git a/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll b/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll
index ccf8cf67ede6d..74d9c86881d52 100644
--- a/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll
+++ b/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll
@@ -32,6 +32,7 @@ entry:
; CHECK-DUMP: <main>:
; CHECK-DUMP: bl 0x8 <main+0x8>
; CHECK-DUMP: <foo>:
+; CHECK-DUMP: paciasp
; `main` doesn't support BTI while `foo` does, so in the binary
; we should see only PAC which is supported by both.
diff --git a/llvm/test/LTO/AArch64/link-sign-return-address.ll b/llvm/test/LTO/AArch64/link-sign-return-address.ll
new file mode 100644
index 0000000000000..b2e70b2b04e08
--- /dev/null
+++ b/llvm/test/LTO/AArch64/link-sign-return-address.ll
@@ -0,0 +1,43 @@
+; Testcase to check that module with different branch-target-enforcement can
+; be mixed.
+;
+; RUN: llvm-as %s -o %t1.bc
+; RUN: llvm-as %p/Inputs/foo.ll -o %t2.bc
+; RUN: llvm-lto -exported-symbol main \
+; RUN: -exported-symbol foo \
+; RUN: -filetype=obj \
+; RUN: %t2.bc %t1.bc \
+; RUN: -o %t1.exe 2>&1
+; RUN: llvm-objdump -d %t1.exe | FileCheck --check-prefix=CHECK-DUMP %s
+; RUN: llvm-readelf -n %t1.exe | FileCheck --allow-empty --check-prefix=CHECK-PROP %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+declare i32 @foo();
+
+define i32 @main() {
+entry:
+ %add = call i32 @foo()
+ ret i32 %add
+}
+
+!llvm.module.flags = !{!0, !1, !2, !3 }
+!0 = !{i32 8, !"branch-target-enforcement", i32 0}
+!1 = !{i32 8, !"sign-return-address", i32 0}
+!2 = !{i32 8, !"sign-return-address-all", i32 0}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 0}
+
+; CHECK-DUMP: <foo>:
+; CHECK-DUMP: paciasp
+; CHECK-DUMP: mov w0, #0x2a
+; CHECK-DUMP: autiasp
+; CHECK-DUMP: ret
+; CHECK-DUMP: <main>:
+; CHECK-DUMP-NOT: paciasp
+; CHECK-DUMP: str x30,
+; CHECK-DUMP: bl 0x14 <main+0x4>
+
+; `main` doesn't support PAC sign-return-address while `foo` does, so in the binary
+; we should not see anything.
+; CHECK-PROP-NOT: Proper ties: aarch64 feature: PAC
\ No newline at end of file
diff --git a/llvm/test/Linker/link-arm-and-thumb.ll b/llvm/test/Linker/link-arm-and-thumb.ll
index a90f2128e4430..37bd8c37f8b5e 100644
--- a/llvm/test/Linker/link-arm-and-thumb.ll
+++ b/llvm/test/Linker/link-arm-and-thumb.ll
@@ -13,11 +13,12 @@ entry:
ret i32 %add
}
-; CHECK: define i32 @main() {
+; CHECK: define i32 @main() [[MAIN_ATTRS:#[0-9]+]]
; CHECK: define i32 @foo(i32 %a, i32 %b) [[ARM_ATTRS:#[0-9]+]]
; CHECK: define i32 @bar(i32 %a, i32 %b) [[THUMB_ATTRS:#[0-9]+]]
-; CHECK: attributes [[ARM_ATTRS]] = { "target-features"="-thumb-mode" }
-; CHECK: attributes [[THUMB_ATTRS]] = { "target-features"="+thumb-mode" }
+; CHECK: attributes [[MAIN_ATTRS]] = { {{.*}} }
+; CHECK: attributes [[ARM_ATTRS]] = { {{.*}} "target-features"="-thumb-mode" }
+; CHECK: attributes [[THUMB_ATTRS]] = { {{.*}} "target-features"="+thumb-mode" }
; STDERR-NOT: warning: Linking two modules of different target triples:
|
|
@llvm/pr-subscribers-clang Author: Dani (DanielKristofKiss) Changes
Full diff: https://github.com/llvm/llvm-project/pull/80640.diff 9 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index c63e4ecc3dcba..36b63d78b06f8 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1044,17 +1044,14 @@ void CodeGenModule::Release() {
llvm::MDString::get(VMContext, "ascii"));
}
- llvm::Triple::ArchType Arch = Context.getTargetInfo().getTriple().getArch();
- if ( Arch == llvm::Triple::arm
- || Arch == llvm::Triple::armeb
- || Arch == llvm::Triple::thumb
- || Arch == llvm::Triple::thumbeb) {
+ llvm::Triple T = Context.getTargetInfo().getTriple();
+ if (T.isARM() || T.isThumb()) {
// The minimum width of an enum in bytes
uint64_t EnumWidth = Context.getLangOpts().ShortEnums ? 1 : 4;
getModule().addModuleFlag(llvm::Module::Error, "min_enum_size", EnumWidth);
}
- if (Arch == llvm::Triple::riscv32 || Arch == llvm::Triple::riscv64) {
+ if (T.isRISCV()) {
StringRef ABIStr = Target.getABI();
llvm::LLVMContext &Ctx = TheModule.getContext();
getModule().addModuleFlag(llvm::Module::Error, "target-abi",
@@ -1127,10 +1124,7 @@ void CodeGenModule::Release() {
getModule().addModuleFlag(llvm::Module::Override,
"tag-stack-memory-buildattr", 1);
- if (Arch == llvm::Triple::thumb || Arch == llvm::Triple::thumbeb ||
- Arch == llvm::Triple::arm || Arch == llvm::Triple::armeb ||
- Arch == llvm::Triple::aarch64 || Arch == llvm::Triple::aarch64_32 ||
- Arch == llvm::Triple::aarch64_be) {
+ if (T.isARM() || T.isThumb() || T.isAArch64()) {
if (LangOpts.BranchTargetEnforcement)
getModule().addModuleFlag(llvm::Module::Min, "branch-target-enforcement",
1);
diff --git a/llvm/include/llvm/IR/AutoUpgrade.h b/llvm/include/llvm/IR/AutoUpgrade.h
index 152f781ffa9b3..c0d96efc54752 100644
--- a/llvm/include/llvm/IR/AutoUpgrade.h
+++ b/llvm/include/llvm/IR/AutoUpgrade.h
@@ -67,7 +67,8 @@ namespace llvm {
void UpgradeSectionAttributes(Module &M);
/// Correct any IR that is relying on old function attribute behavior.
- void UpgradeFunctionAttributes(Function &F);
+ void UpgradeFunctionAttributes(Function &F,
+ bool ModuleMetadataIsMaterialized = false);
/// If the given TBAA tag uses the scalar TBAA format, create a new node
/// corresponding to the upgrade to the struct-path aware TBAA format.
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 5b233fb365fe2..6b335dd9f1f89 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -6708,7 +6708,7 @@ Error BitcodeReader::materialize(GlobalValue *GV) {
}
// Look for functions that rely on old function attribute behavior.
- UpgradeFunctionAttributes(*F);
+ UpgradeFunctionAttributes(*F, true);
// If we've materialized a function set up in "new" debug-info mode, the
// contents just loaded will still be in dbg.value mode. Switch to the new
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 19d80eb9aec0b..e25ac46450cec 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -5155,7 +5155,39 @@ struct StrictFPUpgradeVisitor : public InstVisitor<StrictFPUpgradeVisitor> {
};
} // namespace
-void llvm::UpgradeFunctionAttributes(Function &F) {
+static void
+CopyModuleAttributeToFunction(Function &F, StringRef FnAttrName,
+ StringRef ModAttrName,
+ std::pair<StringRef, StringRef> Values) {
+ Module *M = F.getParent();
+ if (!M)
+ return;
+ if (F.hasFnAttribute(FnAttrName))
+ return;
+ if (const auto *MAttr = mdconst::extract_or_null<ConstantInt>(
+ M->getModuleFlag(ModAttrName))) {
+ if (MAttr->getZExtValue()) {
+ F.addFnAttr(FnAttrName, Values.first);
+ return;
+ }
+ }
+ F.addFnAttr(FnAttrName, Values.second);
+}
+
+static void CopyModuleAttributeToFunction(Function &F, StringRef AttrName) {
+ CopyModuleAttributeToFunction(
+ F, AttrName, AttrName,
+ std::make_pair<StringRef, StringRef>("true", "false"));
+}
+
+static void
+CopyModuleAttributeToFunction(Function &F, StringRef AttrName,
+ std::pair<StringRef, StringRef> Values) {
+ CopyModuleAttributeToFunction(F, AttrName, AttrName, Values);
+}
+
+void llvm::UpgradeFunctionAttributes(Function &F,
+ bool ModuleMetadataIsMaterialized) {
// If a function definition doesn't have the strictfp attribute,
// convert any callsite strictfp attributes to nobuiltin.
if (!F.isDeclaration() && !F.hasFnAttribute(Attribute::StrictFP)) {
@@ -5167,6 +5199,41 @@ void llvm::UpgradeFunctionAttributes(Function &F) {
F.removeRetAttrs(AttributeFuncs::typeIncompatible(F.getReturnType()));
for (auto &Arg : F.args())
Arg.removeAttrs(AttributeFuncs::typeIncompatible(Arg.getType()));
+
+ if (!ModuleMetadataIsMaterialized)
+ return;
+ if (F.isDeclaration())
+ return;
+ Module *M = F.getParent();
+ if (!M)
+ return;
+
+ Triple T(M->getTargetTriple());
+ // Convert module level attributes to function level attributes because
+ // after merging modules the attributes might change and would have different
+ // effect on the functions as the original module would have.
+ if (T.isThumb() || T.isARM() || T.isAArch64()) {
+ if (!F.hasFnAttribute("sign-return-address")) {
+ StringRef SignType = "none";
+ if (const auto *Sign = mdconst::extract_or_null<ConstantInt>(
+ M->getModuleFlag("sign-return-address")))
+ if (Sign->getZExtValue())
+ SignType = "non-leaf";
+
+ if (const auto *SignAll = mdconst::extract_or_null<ConstantInt>(
+ M->getModuleFlag("sign-return-address-all")))
+ if (SignAll->getZExtValue())
+ SignType = "all";
+
+ F.addFnAttr("sign-return-address", SignType);
+ }
+ CopyModuleAttributeToFunction(F, "branch-target-enforcement");
+ CopyModuleAttributeToFunction(F, "branch-protection-pauth-lr");
+ CopyModuleAttributeToFunction(F, "guarded-control-stack");
+ CopyModuleAttributeToFunction(
+ F, "sign-return-address-key",
+ std::make_pair<StringRef, StringRef>("b_key", "a_key"));
+ }
}
static bool isOldLoopArgument(Metadata *MD) {
diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index 8cc0f7fb90991..47d5a39c9f746 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -1606,6 +1606,10 @@ Error IRLinker::run() {
// Loop over all of the linked values to compute type mappings.
computeTypeMapping();
+ // Update function attributes before copy them to destation module.
+ for (Function &F : SrcM->getFunctionList())
+ UpgradeFunctionAttributes(F, true);
+
std::reverse(Worklist.begin(), Worklist.end());
while (!Worklist.empty()) {
GlobalValue *GV = Worklist.back();
diff --git a/llvm/test/Bitcode/upgrade-arc-runtime-calls.ll b/llvm/test/Bitcode/upgrade-arc-runtime-calls.ll
index 19f25f98953fa..d2edec18d55e5 100644
--- a/llvm/test/Bitcode/upgrade-arc-runtime-calls.ll
+++ b/llvm/test/Bitcode/upgrade-arc-runtime-calls.ll
@@ -55,7 +55,7 @@ unwindBlock:
// Check that auto-upgrader converts function calls to intrinsic calls. Note that
// the auto-upgrader doesn't touch invoke instructions.
-// ARC: define void @testRuntimeCalls(ptr %[[A:.*]], ptr %[[B:.*]], ptr %[[C:.*]], ptr %[[D:.*]], ptr %[[E:.*]]) personality
+// ARC: define void @testRuntimeCalls(ptr %[[A:.*]], ptr %[[B:.*]], ptr %[[C:.*]], ptr %[[D:.*]], ptr %[[E:.*]]) #0 personality
// ARC: %[[V0:.*]] = tail call ptr @llvm.objc.autorelease(ptr %[[A]])
// ARC-NEXT: tail call void @llvm.objc.autoreleasePoolPop(ptr %[[A]])
// ARC-NEXT: %[[V1:.*]] = tail call ptr @llvm.objc.autoreleasePoolPush()
@@ -88,7 +88,7 @@ unwindBlock:
// ARC-NEXT: tail call void @llvm.objc.arc.annotation.bottomup.bbend(ptr %[[B]], ptr %[[C]])
// ARC-NEXT: invoke void @objc_autoreleasePoolPop(ptr %[[A]])
-// NOUPGRADE: define void @testRuntimeCalls(ptr %[[A:.*]], ptr %[[B:.*]], ptr %[[C:.*]], ptr %[[D:.*]], ptr %[[E:.*]]) personality
+// NOUPGRADE: define void @testRuntimeCalls(ptr %[[A:.*]], ptr %[[B:.*]], ptr %[[C:.*]], ptr %[[D:.*]], ptr %[[E:.*]]) #0 personality
// NOUPGRADE: %[[V0:.*]] = tail call ptr @objc_autorelease(ptr %[[A]])
// NOUPGRADE-NEXT: tail call void @objc_autoreleasePoolPop(ptr %[[A]])
// NOUPGRADE-NEXT: %[[V1:.*]] = tail call ptr @objc_autoreleasePoolPush()
diff --git a/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll b/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll
index ccf8cf67ede6d..74d9c86881d52 100644
--- a/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll
+++ b/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll
@@ -32,6 +32,7 @@ entry:
; CHECK-DUMP: <main>:
; CHECK-DUMP: bl 0x8 <main+0x8>
; CHECK-DUMP: <foo>:
+; CHECK-DUMP: paciasp
; `main` doesn't support BTI while `foo` does, so in the binary
; we should see only PAC which is supported by both.
diff --git a/llvm/test/LTO/AArch64/link-sign-return-address.ll b/llvm/test/LTO/AArch64/link-sign-return-address.ll
new file mode 100644
index 0000000000000..b2e70b2b04e08
--- /dev/null
+++ b/llvm/test/LTO/AArch64/link-sign-return-address.ll
@@ -0,0 +1,43 @@
+; Testcase to check that module with different branch-target-enforcement can
+; be mixed.
+;
+; RUN: llvm-as %s -o %t1.bc
+; RUN: llvm-as %p/Inputs/foo.ll -o %t2.bc
+; RUN: llvm-lto -exported-symbol main \
+; RUN: -exported-symbol foo \
+; RUN: -filetype=obj \
+; RUN: %t2.bc %t1.bc \
+; RUN: -o %t1.exe 2>&1
+; RUN: llvm-objdump -d %t1.exe | FileCheck --check-prefix=CHECK-DUMP %s
+; RUN: llvm-readelf -n %t1.exe | FileCheck --allow-empty --check-prefix=CHECK-PROP %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+declare i32 @foo();
+
+define i32 @main() {
+entry:
+ %add = call i32 @foo()
+ ret i32 %add
+}
+
+!llvm.module.flags = !{!0, !1, !2, !3 }
+!0 = !{i32 8, !"branch-target-enforcement", i32 0}
+!1 = !{i32 8, !"sign-return-address", i32 0}
+!2 = !{i32 8, !"sign-return-address-all", i32 0}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 0}
+
+; CHECK-DUMP: <foo>:
+; CHECK-DUMP: paciasp
+; CHECK-DUMP: mov w0, #0x2a
+; CHECK-DUMP: autiasp
+; CHECK-DUMP: ret
+; CHECK-DUMP: <main>:
+; CHECK-DUMP-NOT: paciasp
+; CHECK-DUMP: str x30,
+; CHECK-DUMP: bl 0x14 <main+0x4>
+
+; `main` doesn't support PAC sign-return-address while `foo` does, so in the binary
+; we should not see anything.
+; CHECK-PROP-NOT: Proper ties: aarch64 feature: PAC
\ No newline at end of file
diff --git a/llvm/test/Linker/link-arm-and-thumb.ll b/llvm/test/Linker/link-arm-and-thumb.ll
index a90f2128e4430..37bd8c37f8b5e 100644
--- a/llvm/test/Linker/link-arm-and-thumb.ll
+++ b/llvm/test/Linker/link-arm-and-thumb.ll
@@ -13,11 +13,12 @@ entry:
ret i32 %add
}
-; CHECK: define i32 @main() {
+; CHECK: define i32 @main() [[MAIN_ATTRS:#[0-9]+]]
; CHECK: define i32 @foo(i32 %a, i32 %b) [[ARM_ATTRS:#[0-9]+]]
; CHECK: define i32 @bar(i32 %a, i32 %b) [[THUMB_ATTRS:#[0-9]+]]
-; CHECK: attributes [[ARM_ATTRS]] = { "target-features"="-thumb-mode" }
-; CHECK: attributes [[THUMB_ATTRS]] = { "target-features"="+thumb-mode" }
+; CHECK: attributes [[MAIN_ATTRS]] = { {{.*}} }
+; CHECK: attributes [[ARM_ATTRS]] = { {{.*}} "target-features"="-thumb-mode" }
+; CHECK: attributes [[THUMB_ATTRS]] = { {{.*}} "target-features"="+thumb-mode" }
; STDERR-NOT: warning: Linking two modules of different target triples:
|
DanielKristofKiss
requested review from
pcc,
nickdesaulniers,
momchil-velikov,
pirama-arumuga-nainar and
MaskRay
|
CI failed on Windows with antivirus issue on a not related test, passed on linux. |
sign-return-address and similar module attributes should be propagated to the function level before got merged because module flags may contradict and this information is not recoverable. Generated code will match with the normal linking flow.
DanielKristofKiss
force-pushed
the
autoupgrade
branch
from
3f4eb61 to
7a9286d
Compare
|
Updates with comments in #80642 |
|
Patch LGTM but I wanted to follow up on the point about clang code gen from the other thread.
Right, I do recall issues with functions that have been synthesized by the compiler lacking certain function attributes...I thought we had another way of solving those (that was not AutoUpgrade). I think that's the point of
Does auto upgrade run as a pass in the default pipelines? I thought it was just for helping folks move from one version of IR to another (for .bc or .ll files). |
backend handles the model vs function attribute e.g. llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.cpp::GetSignReturnAddress So far the module attribute is the contract between the fronted/backend and I know e.g. Rust relies on that too. Clang also just sets the module flags today if the flag is not function attribute, but I have a patch for that too but I need to add the
That is the change that I added here, llvm/lib/Linker/IRMover.cpp |
nickdesaulniers
approved these changes
Feb 22, 2024
Ah! Ok I was missing that either are already supported; I was thinking that function attributes would be new, but it looks like there is pre-existing support as you cite.
You can CC for review on that, too, when it's ready. |
DanielKristofKiss
added a commit
to DanielKristofKiss/llvm-project
that referenced
this pull request
Feb 23, 2024
…s. (llvm#80640) `sign-return-address` and similar module attributes should be propagated to the function level before modules got merged because module flags may contradict and this information is not recoverable. Generated code will match with the normal linking flow.
|
FYI, this has caused test failures: https://lab.llvm.org/buildbot/#/builders/188/builds/42267 |
|
This also causes an unreasonably large compile-time regression for what it does: https://llvm-compile-time-tracker.com/compare.php?from=354401f8d3dc08ed41895d03a12a122e9cc0482c&to=531e8c26b3f2626e7f1a997e0e8b61d67d10aded&stat=instructions:u Surely upgrading attributes should not take 0.5% compile-time during the clang thin link, especially on a target where these attributes do not even exist? This logic should probably not be inside UpgradeFunctionAttributes() -- you'll want to structure this the other way around and first determine which module attributes (if any) have to be applied, and then apply them to all functions. |
|
Also, should this upgrade really run during normal bitcode reading? It makes little sense to me that current bitcode will no longer round-trip between llvm-as / llvm-dis after this change. Should the change here be limited to IRMover only? |
DanielKristofKiss
added a commit
to DanielKristofKiss/llvm-project
that referenced
this pull request
Feb 23, 2024
|
Original change is reverted. |
|
#82763 is opened as I can't reopen this one. |
DanielKristofKiss
added a commit
to DanielKristofKiss/llvm-project
that referenced
this pull request
Feb 28, 2024
DanielKristofKiss
added a commit
to DanielKristofKiss/llvm-project
that referenced
this pull request
Feb 28, 2024
DanielKristofKiss
added a commit
that referenced
this pull request
Mar 4, 2024
…s. (#82763) sign-return-address and similar module attributes should be propagated to the function level before got merged because module flags may contradict and this information is not recoverable. Generated code will match with the normal linking flow. Refactored version of (#80640). Run the attribute copy only during IRMove.
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Mar 5, 2024
…s. (llvm#82763) sign-return-address and similar module attributes should be propagated to the function level before got merged because module flags may contradict and this information is not recoverable. Generated code will match with the normal linking flow. Refactored version of (llvm#80640). Run the attribute copy only during IRMove. (cherry picked from commit ded5de1)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
sign-return-addressand similar module attributes should be propagated tothe function level before got merged because module flags may contradict and
this information is not recoverable.
Generated code will match with the normal linking flow.