[ARM][AArch64] BTI,GCS,PAC Module flag update. #86212

DanielKristofKiss · 2024-03-21T22:38:48Z

Module flag is used to indicate the feature to be propagated to the function. As now the frontend emits all attributes accordingly let's help the auto upgrade to only do work when old and new bitcodes are merged.

Depends on #82819 and #86031

llvmbot · 2024-03-21T22:39:15Z

@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-lto
@llvm/pr-subscribers-backend-arm
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: Daniel Kiss (DanielKristofKiss)

Changes

Module flag is used to indicate the feature to be propagated to the function. As now the frontend emits all attributes accordingly let's help the auto upgrade to only do work when old and new bitcodes are merged.

Depends on #82819 and #86031

Patch is 26.38 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86212.diff

20 Files Affected:

(modified) clang/lib/CodeGen/CodeGenModule.cpp (+13-6)
(modified) clang/test/CodeGen/aarch64-sign-return-address.c (+6-6)
(modified) clang/test/CodeGen/arm-branch-protection-attr-2.c (+4-4)
(modified) clang/test/Frontend/arm-ignore-branch-protection-option.c (+1-1)
(added) lld/test/ELF/lto/aarch64_inline.ll (+73)
(modified) llvm/include/llvm/IR/AutoUpgrade.h (+3)
(modified) llvm/lib/AsmParser/LLParser.cpp (+1)
(modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+2)
(modified) llvm/lib/IR/AutoUpgrade.cpp (+100)
(modified) llvm/lib/Linker/IRMover.cpp (+10)
(modified) llvm/lib/Target/ARM/ARMAsmPrinter.cpp (+1-1)
(modified) llvm/lib/Transforms/IPO/LowerTypeTests.cpp (+1-1)
(modified) llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll (+1-1)
(removed) llvm/test/LTO/AArch64/Inputs/foo.ll (-16)
(added) llvm/test/LTO/AArch64/TestInputs/bar.ll (+35)
(added) llvm/test/LTO/AArch64/TestInputs/foo.ll (+38)
(added) llvm/test/LTO/AArch64/TestInputs/old.ll (+46)
(modified) llvm/test/LTO/AArch64/link-branch-target-enforcement.ll (+4-1)
(added) llvm/test/LTO/AArch64/link-sign-return-address.ll (+102)
(modified) llvm/test/Linker/link-arm-and-thumb.ll (+3-3)

diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index cb153066b28dd1..1acc0510256268 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1173,22 +1173,29 @@ void CodeGenModule::Release() {
                               "tag-stack-memory-buildattr", 1);
 
   if (T.isARM() || T.isThumb() || T.isAArch64()) {
+    // Previously 1 is used and meant for the backed to derive the function
+    // attribute form it. 2 now means function attributes already set for all
+    // functions in this module, so no need to propagate those from the module
+    // flag. Value is only used in case of LTO module merge because the backend
+    // will see all required function attribute set already. Value is used
+    // before modules got merged. Any posive value means the feature is active
+    // and required binary markings need to be emit accordingly.
     if (LangOpts.BranchTargetEnforcement)
       getModule().addModuleFlag(llvm::Module::Min, "branch-target-enforcement",
-                                1);
+                                2);
     if (LangOpts.BranchProtectionPAuthLR)
       getModule().addModuleFlag(llvm::Module::Min, "branch-protection-pauth-lr",
-                                1);
+                                2);
     if (LangOpts.GuardedControlStack)
-      getModule().addModuleFlag(llvm::Module::Min, "guarded-control-stack", 1);
+      getModule().addModuleFlag(llvm::Module::Min, "guarded-control-stack", 2);
     if (LangOpts.hasSignReturnAddress())
-      getModule().addModuleFlag(llvm::Module::Min, "sign-return-address", 1);
+      getModule().addModuleFlag(llvm::Module::Min, "sign-return-address", 2);
     if (LangOpts.isSignReturnAddressScopeAll())
       getModule().addModuleFlag(llvm::Module::Min, "sign-return-address-all",
-                                1);
+                                2);
     if (!LangOpts.isSignReturnAddressWithAKey())
       getModule().addModuleFlag(llvm::Module::Min,
-                                "sign-return-address-with-bkey", 1);
+                                "sign-return-address-with-bkey", 2);
   }
 
   if (CodeGenOpts.StackClashProtector)
diff --git a/clang/test/CodeGen/aarch64-sign-return-address.c b/clang/test/CodeGen/aarch64-sign-return-address.c
index 8bc54b1a56c38c..35c56889e07071 100644
--- a/clang/test/CodeGen/aarch64-sign-return-address.c
+++ b/clang/test/CodeGen/aarch64-sign-return-address.c
@@ -22,17 +22,17 @@
 // NONE-NOT:  !"branch-target-enforcement"
 // ALL-NOT:   !"branch-target-enforcement"
 // PART-NOT:  !"branch-target-enforcement"
-// BTE:       !{i32 8, !"branch-target-enforcement", i32 1}
+// BTE:       !{i32 8, !"branch-target-enforcement", i32 2}
 // B-KEY-NOT: !"branch-target-enforcement"
 
 // NONE-NOT:  !"sign-return-address"
-// ALL:   !{i32 8, !"sign-return-address", i32 1}
-// PART:  !{i32 8, !"sign-return-address", i32 1}
+// ALL:   !{i32 8, !"sign-return-address", i32 2}
+// PART:  !{i32 8, !"sign-return-address", i32 2}
 // BTE-NOT:   !"sign-return-address"
-// B-KEY: !{i32 8, !"sign-return-address", i32 1}
+// B-KEY: !{i32 8, !"sign-return-address", i32 2}
 
 // NONE-NOT:  !"sign-return-address-all"
-// ALL:   !{i32 8, !"sign-return-address-all", i32 1}
+// ALL:   !{i32 8, !"sign-return-address-all", i32 2}
 // PART-NOT:  !"sign-return-address-all"
 // BTE-NOT:   !"sign-return-address-all"
 // B-KEY-NOT: !"sign-return-address-all"
@@ -41,6 +41,6 @@
 // ALL-NOT:   !"sign-return-address-with-bkey"
 // PART-NOT:  !"sign-return-address-with-bkey"
 // BTE-NOT:   !"sign-return-address-with-bkey"
-// B-KEY: !{i32 8, !"sign-return-address-with-bkey", i32 1}
+// B-KEY: !{i32 8, !"sign-return-address-with-bkey", i32 2}
 
 void foo() {}
diff --git a/clang/test/CodeGen/arm-branch-protection-attr-2.c b/clang/test/CodeGen/arm-branch-protection-attr-2.c
index 1f3c00873043e8..741c0026c4d05c 100644
--- a/clang/test/CodeGen/arm-branch-protection-attr-2.c
+++ b/clang/test/CodeGen/arm-branch-protection-attr-2.c
@@ -18,16 +18,16 @@
 // NONE-NOT: !"branch-target-enforcement"
 // PART-NOT: !"branch-target-enforcement"
 // ALL-NOT:  !"branch-target-enforcement"
-// BTE:      !{i32 8, !"branch-target-enforcement", i32 1}
+// BTE:      !{i32 8, !"branch-target-enforcement", i32 2}
 
 // NONE-NOT: !"sign-return-address"
-// PART:     !{i32 8, !"sign-return-address", i32 1}
-// ALL:      !{i32 8, !"sign-return-address", i32 1}
+// PART:     !{i32 8, !"sign-return-address", i32 2}
+// ALL:      !{i32 8, !"sign-return-address", i32 2}
 // BTE-NOT:  !"sign-return-address"
 
 // NONE-NOT: !"sign-return-address-all", i32 0}
 // PART-NOT: !"sign-return-address-all", i32 0}
-// ALL:      !{i32 8, !"sign-return-address-all", i32 1}
+// ALL:      !{i32 8, !"sign-return-address-all", i32 2}
 // BTE-NOT:  !"sign-return-address-all", i32 0}
 
 void foo() {}
diff --git a/clang/test/Frontend/arm-ignore-branch-protection-option.c b/clang/test/Frontend/arm-ignore-branch-protection-option.c
index 99a2accef3ae2f..45bdb37f5ed1a3 100644
--- a/clang/test/Frontend/arm-ignore-branch-protection-option.c
+++ b/clang/test/Frontend/arm-ignore-branch-protection-option.c
@@ -15,4 +15,4 @@ __attribute__((target("arch=cortex-m0"))) void f() {}
 // CHECK-NOT:  attributes { {{.*}} "branch-target-enforcement"
 
 /// Check that there are branch protection module attributes despite the warning.
-// CHECK: !{i32 8, !"branch-target-enforcement", i32 1}
+// CHECK: !{i32 8, !"branch-target-enforcement", i32 2}
diff --git a/lld/test/ELF/lto/aarch64_inline.ll b/lld/test/ELF/lto/aarch64_inline.ll
new file mode 100644
index 00000000000000..67fa74e528dee9
--- /dev/null
+++ b/lld/test/ELF/lto/aarch64_inline.ll
@@ -0,0 +1,73 @@
+; REQUIRES: aarch64
+;; Test verifies inlining happens cross module when module flags are upgraded
+;; by the thin-lto linker/IRMover.
+;; Regression test for #82763
+
+; RUN: split-file %s %t
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -unified-lto %t/foo.s -o %t/foo.o
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -unified-lto %t/bar.s -o %t/bar.o
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -unified-lto %t/main.s -o %t/main.o
+; RUN: ld.lld -O2 --lto=thin --entry=main %t/main.o %t/foo.o %t/bar.o -o %t/exe
+; RUN: llvm-objdump -d %t/exe | FileCheck %s
+
+
+; CHECK-LABEL:  <main>:
+; CHECK-NEXT:         pacibsp
+; CHECK-NEXT:         mov     w0, #0x23
+; CHECK-NEXT:         autibsp
+; CHECK-NEXT:         ret
+
+;--- foo.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"
+
+define dso_local noundef i32 @foo() local_unnamed_addr #0 {
+entry:
+  ret i32 34
+}
+
+attributes #0 = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) }
+!llvm.module.flags = !{!0, !1, !2, !3 }
+!0 = !{i32 8, !"branch-target-enforcement", i32 1}
+!1 = !{i32 8, !"sign-return-address", i32 1}
+!2 = !{i32 8, !"sign-return-address-all", i32 1}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 1}
+
+;--- bar.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"
+
+define dso_local noundef i32 @bar() local_unnamed_addr #0 {
+entry:
+  ret i32 1
+}
+
+attributes #0 = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) "branch-target-enforcement" "sign-return-address"="all" "sign-return-address-key"="b_key" }
+!llvm.module.flags = !{!0, !1, !2, !3 }
+!0 = !{i32 8, !"branch-target-enforcement", i32 2}
+!1 = !{i32 8, !"sign-return-address", i32 2}
+!2 = !{i32 8, !"sign-return-address-all", i32 2}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 2}
+
+;--- main.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();
+declare i32 @bar();
+
+define i32 @main() #0 {
+entry:
+  %1 = call i32 @foo()
+  %2 = call i32 @bar()
+  %3 = add i32 %1, %2
+  ret i32 %3
+}
+
+attributes #0 = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) }
+
+!llvm.module.flags = !{!0, !1, !2, !3 }
+!0 = !{i32 8, !"branch-target-enforcement", i32 1}
+!1 = !{i32 8, !"sign-return-address", i32 1}
+!2 = !{i32 8, !"sign-return-address-all", i32 1}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 1}
diff --git a/llvm/include/llvm/IR/AutoUpgrade.h b/llvm/include/llvm/IR/AutoUpgrade.h
index 152f781ffa9b30..1ef32bcb121bec 100644
--- a/llvm/include/llvm/IR/AutoUpgrade.h
+++ b/llvm/include/llvm/IR/AutoUpgrade.h
@@ -88,6 +88,9 @@ namespace llvm {
   /// info. Return true if module is modified.
   bool UpgradeDebugInfo(Module &M);
 
+  /// Copies module attributes to the functions in the module.
+  void CopyModuleAttrToFunctions(Module &M);
+
   /// Check whether a string looks like an old loop attachment tag.
   inline bool mayBeOldLoopAttachmentTag(StringRef Name) {
     return Name.starts_with("llvm.vectorizer.");
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index f0be021668afa7..299e0d9f29953d 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -408,6 +408,7 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
 
   UpgradeModuleFlags(*M);
   UpgradeSectionAttributes(*M);
+  CopyModuleAttrToFunctions(*M);
 
   if (!Slots)
     return false;
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 3fc8141381c623..a07b4aa0b5cd5d 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -6927,6 +6927,8 @@ Error BitcodeReader::materializeModule() {
 
   UpgradeARCRuntime(*TheModule);
 
+  CopyModuleAttrToFunctions(*TheModule);
+
   return Error::success();
 }
 
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index a44f6af4162f03..8a671657c9c9ec 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -5250,6 +5250,106 @@ void llvm::UpgradeFunctionAttributes(Function &F) {
     Arg.removeAttrs(AttributeFuncs::typeIncompatible(Arg.getType()));
 }
 
+// Check if the module attribute is present and set to one.
+static bool isModuleAttributeOne(Module &M, const StringRef &ModAttr) {
+  const auto *Attr =
+      mdconst::extract_or_null<ConstantInt>(M.getModuleFlag(ModAttr));
+  return Attr && Attr->isOne();
+}
+
+// Check if the module attribute is present and set to two.
+static bool isModuleAttributeTwo(Module &M, const StringRef &ModAttr) {
+  const auto *Attr =
+      mdconst::extract_or_null<ConstantInt>(M.getModuleFlag(ModAttr));
+  return Attr && Attr->getZExtValue() == 2;
+}
+
+// Check if the function attribute is not present and set it.
+static void SetFunctionAttrIfNotSet(Function &F, StringRef FnAttrName,
+                                    StringRef Value) {
+  if (!F.hasFnAttribute(FnAttrName))
+    F.addFnAttr(FnAttrName, Value);
+}
+
+// Check if the function attribute is not present and set it if needed.
+// If the attribute is "false" then removes it.
+// If the attribute is "true" resets it to a valueless attribute.
+static void ConvertFunctionAttr(Function &F, bool Set, StringRef FnAttrName) {
+  if (!F.hasFnAttribute(FnAttrName)) {
+    if (Set)
+      F.addFnAttr(FnAttrName);
+  } else {
+    auto A = F.getFnAttribute(FnAttrName);
+    if ("false" == A.getValueAsString())
+      F.removeFnAttr(FnAttrName);
+    else if ("true" == A.getValueAsString()) {
+      F.removeFnAttr(FnAttrName);
+      F.addFnAttr(FnAttrName);
+    }
+  }
+}
+
+void llvm::CopyModuleAttrToFunctions(Module &M) {
+  Triple T(M.getTargetTriple());
+  if (!T.isThumb() && !T.isARM() && !T.isAArch64())
+    return;
+
+  if (isModuleAttributeTwo(M, "branch-target-enforcement"))
+    return;
+  if (isModuleAttributeTwo(M, "branch-protection-pauth-lr"))
+    return;
+  if (isModuleAttributeTwo(M, "guarded-control-stack"))
+    return;
+  if (isModuleAttributeTwo(M, "sign-return-address"))
+    return;
+
+  bool BTE = isModuleAttributeOne(M, "branch-target-enforcement");
+  bool BPPLR = isModuleAttributeOne(M, "branch-protection-pauth-lr");
+  bool GCS = isModuleAttributeOne(M, "guarded-control-stack");
+  bool SRA = isModuleAttributeOne(M, "sign-return-address");
+
+  StringRef SignTypeValue = "non-leaf";
+  if (SRA && isModuleAttributeOne(M, "sign-return-address-all"))
+    SignTypeValue = "all";
+
+  StringRef SignKeyValue = "a_key";
+  if (SRA && isModuleAttributeOne(M, "sign-return-address-with-bkey"))
+    SignKeyValue = "b_key";
+
+  for (Function &F : M.getFunctionList()) {
+    if (F.isDeclaration())
+      continue;
+
+    if (SRA) {
+      SetFunctionAttrIfNotSet(F, "sign-return-address", SignTypeValue);
+      SetFunctionAttrIfNotSet(F, "sign-return-address-key", SignKeyValue);
+    } else {
+      if (auto A = F.getFnAttribute("sign-return-address");
+          A.isValid() && "none" == A.getValueAsString()) {
+        F.removeFnAttr("sign-return-address");
+        F.removeFnAttr("sign-return-address-key");
+      }
+    }
+    ConvertFunctionAttr(F, BTE, "branch-target-enforcement");
+    ConvertFunctionAttr(F, BPPLR, "branch-protection-pauth-lr");
+    ConvertFunctionAttr(F, GCS, "guarded-control-stack");
+  }
+
+  if (BTE)
+    M.setModuleFlag(llvm::Module::Min, "branch-target-enforcement", 2);
+  if (BPPLR)
+    M.setModuleFlag(llvm::Module::Min, "branch-protection-pauth-lr", 2);
+  if (GCS)
+    M.setModuleFlag(llvm::Module::Min, "guarded-control-stack", 2);
+  if (SRA) {
+    M.setModuleFlag(llvm::Module::Min, "sign-return-address", 2);
+    if (isModuleAttributeOne(M, "sign-return-address-all"))
+      M.setModuleFlag(llvm::Module::Min, "sign-return-address-all", 2);
+    if (isModuleAttributeOne(M, "sign-return-address-with-bkey"))
+      M.setModuleFlag(llvm::Module::Min, "sign-return-address-with-bkey", 2);
+  }
+}
+
 static bool isOldLoopArgument(Metadata *MD) {
   auto *T = dyn_cast_or_null<MDTuple>(MD);
   if (!T)
diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index a7e6db82e5c23c..b019cd201313df 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -1623,6 +1623,11 @@ Error IRLinker::run() {
   // Loop over all of the linked values to compute type mappings.
   computeTypeMapping();
 
+  // 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.
+  CopyModuleAttrToFunctions(*SrcM);
+
   std::reverse(Worklist.begin(), Worklist.end());
   while (!Worklist.empty()) {
     GlobalValue *GV = Worklist.back();
@@ -1787,6 +1792,11 @@ IRMover::IRMover(Module &M) : Composite(M) {
   for (const auto *MD : StructTypes.getVisitedMetadata()) {
     SharedMDs[MD].reset(const_cast<MDNode *>(MD));
   }
+
+  // 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.
+  CopyModuleAttrToFunctions(M);
 }
 
 Error IRMover::move(std::unique_ptr<Module> Src,
diff --git a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
index 642739a29d6b06..fe9f207a53e436 100644
--- a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
+++ b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
@@ -793,7 +793,7 @@ void ARMAsmPrinter::emitAttributes() {
 
     auto *BTIValue = mdconst::extract_or_null<ConstantInt>(
         SourceModule->getModuleFlag("branch-target-enforcement"));
-    if (BTIValue && BTIValue->isOne()) {
+    if (BTIValue && !BTIValue->isZero()) {
       // If "+pacbti" is used as an architecture extension,
       // Tag_BTI_extension is emitted in
       // ARMTargetStreamer::emitTargetAttributes().
diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
index 633fcb3314c42f..943360f725ce1f 100644
--- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -1211,7 +1211,7 @@ bool LowerTypeTestsModule::hasBranchTargetEnforcement() {
     // the module flags.
     if (const auto *BTE = mdconst::extract_or_null<ConstantInt>(
           M.getModuleFlag("branch-target-enforcement")))
-      HasBranchTargetEnforcement = (BTE->getZExtValue() != 0);
+      HasBranchTargetEnforcement = !BTE->isZero();
     else
       HasBranchTargetEnforcement = 0;
   }
diff --git a/llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll b/llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll
index 6bfaf3bb520a0a..77c3424f4abf30 100644
--- a/llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll
+++ b/llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll
@@ -94,5 +94,5 @@ attributes #1 = { minsize nofree norecurse nounwind optsize "sign-return-address
 !llvm.module.flags = !{!0, !1, !2}
 
 !0 = !{i32 8, !"branch-target-enforcement", i32 0}
-!1 = !{i32 8, !"sign-return-address", i32 1}
+!1 = !{i32 8, !"sign-return-address", i32 2}
 !2 = !{i32 8, !"sign-return-address-all", i32 0}
diff --git a/llvm/test/LTO/AArch64/Inputs/foo.ll b/llvm/test/LTO/AArch64/Inputs/foo.ll
deleted file mode 100644
index 961b0d4e7997e1..00000000000000
--- a/llvm/test/LTO/AArch64/Inputs/foo.ll
+++ /dev/null
@@ -1,16 +0,0 @@
-target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
-target triple = "aarch64-unknown-linux-gnu"
-
-define dso_local i32 @foo() #0 {
-entry:
-  ret i32 42
-}
-
-attributes #0 = { noinline nounwind optnone uwtable }
-
-!llvm.module.flags = !{!0, !1, !2, !3}
-
-!0 = !{i32 8, !"branch-target-enforcement", i32 1}
-!1 = !{i32 8, !"sign-return-address", i32 1}
-!2 = !{i32 8, !"sign-return-address-all", i32 1}
-!3 = !{i32 8, !"sign-return-address-with-bkey", i32 1}
diff --git a/llvm/test/LTO/AArch64/TestInputs/bar.ll b/llvm/test/LTO/AArch64/TestInputs/bar.ll
new file mode 100644
index 00000000000000..e8a92767c8d7a3
--- /dev/null
+++ b/llvm/test/LTO/AArch64/TestInputs/bar.ll
@@ -0,0 +1,35 @@
+; This file contains the new semantic of the branch-target-enforcement, sign-return-address.
+; Used for test mixing a mixed link case and also verify the import too in llc.
+
+; RUN: llc %s -o - | FileCheck %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"
+
+define dso_local void @bar() #0 {
+entry:
+  ret void
+}
+; CHECK-LABEL: bar:
+; CHECK-NOT:       hint
+; CHECK-NOT:       bti
+; CHECK:           ret
+
+define dso_local void @baz() #1 {
+entry:
+  ret void
+}
+
+; CHECK-LABEL: baz:
+; CHECK:           hint
+; CHECK:           ret
+
+attributes #0 = { noinline nounwind optnone uwtable }
+attributes #1 = { noinline nounwind optnone uwtable "branch-target-enforcement" }
+
+!llvm.module.flags = !{!0, !1, !2, !3}
+
+!0 = !{i32 8, !"branch-target-enforcement", i32 2}
+!1 = !{i32 8, !"sign-return-address", i32 2}
+!2 = !{i32 8, !"sign-return-address-all", i32 2}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 2}
diff --git a/llvm/test/LTO/AArch64/TestInputs/foo.ll b/llvm/test/LTO/AArch64/TestInputs/foo.ll
new file mode 100644
index 00000000000000..7a4cb2ce5ade78
--- /dev/null
+++ b/llvm/test/LTO/AArch64/TestInputs/foo.ll
@@ -0,0 +1,38 @@
+; This file contains the previous semantic of the branch-target-enforcement, sign-return-address.
+; Used for test mixing a mixed link case and also verify the import too in llc.
+
+; RUN: llc %s -o - | FileCheck %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"
+
+define i32 @foo() #0 {
+entry:
+  ret i32 42
+}
+
+; CHECK-LABEL: foo:
+; CHECK:           hint    #27
+; CHECK:           mov
+; CHECK:           hint    #31
+; CHECK:           ret
+
+define i32 @fiz() #1 {
+entry:
+  ret i32 43
+}
+
+; CHECK-LABEL: fiz:
+; CHECK-NOT:       hint
+; CHECK-NOT:       bti
+; CHECK:           ret
+
+attributes #0 = { noinline nounwind optnone uwtable }
+attributes #1 = { noinline nounwind optnone uwtable "branch-target-enforcement"="false" "sign-return-address"="none" }
+
+!llvm.module.flags = !{!0, !1, !2, !3}
+
+!0 = !{i32 8, !"branch-target-enforcement", i32 1}
+!1 = !{i32 8, !"sign-return-address", i32 1}
+!2 = !{i32 8, !"sign-return-address-all", i32 1}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 1}
diff --git a/llvm/test/LTO/AArch64/TestInputs/old.ll b/llvm/test/LTO/AArch64/TestInputs/old.ll
new file mode 100644
index 00000000000000..119ea6fabbd70a
--- /d...
[truncated]

llvmbot · 2024-03-21T22:39:15Z

@llvm/pr-subscribers-clang-codegen

Author: Daniel Kiss (DanielKristofKiss)

Changes

Module flag is used to indicate the feature to be propagated to the function. As now the frontend emits all attributes accordingly let's help the auto upgrade to only do work when old and new bitcodes are merged.

Depends on #82819 and #86031

Patch is 26.38 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86212.diff

20 Files Affected:

(modified) clang/lib/CodeGen/CodeGenModule.cpp (+13-6)
(modified) clang/test/CodeGen/aarch64-sign-return-address.c (+6-6)
(modified) clang/test/CodeGen/arm-branch-protection-attr-2.c (+4-4)
(modified) clang/test/Frontend/arm-ignore-branch-protection-option.c (+1-1)
(added) lld/test/ELF/lto/aarch64_inline.ll (+73)
(modified) llvm/include/llvm/IR/AutoUpgrade.h (+3)
(modified) llvm/lib/AsmParser/LLParser.cpp (+1)
(modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+2)
(modified) llvm/lib/IR/AutoUpgrade.cpp (+100)
(modified) llvm/lib/Linker/IRMover.cpp (+10)
(modified) llvm/lib/Target/ARM/ARMAsmPrinter.cpp (+1-1)
(modified) llvm/lib/Transforms/IPO/LowerTypeTests.cpp (+1-1)
(modified) llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll (+1-1)
(removed) llvm/test/LTO/AArch64/Inputs/foo.ll (-16)
(added) llvm/test/LTO/AArch64/TestInputs/bar.ll (+35)
(added) llvm/test/LTO/AArch64/TestInputs/foo.ll (+38)
(added) llvm/test/LTO/AArch64/TestInputs/old.ll (+46)
(modified) llvm/test/LTO/AArch64/link-branch-target-enforcement.ll (+4-1)
(added) llvm/test/LTO/AArch64/link-sign-return-address.ll (+102)
(modified) llvm/test/Linker/link-arm-and-thumb.ll (+3-3)

diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index cb153066b28dd1..1acc0510256268 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1173,22 +1173,29 @@ void CodeGenModule::Release() {
                               "tag-stack-memory-buildattr", 1);
 
   if (T.isARM() || T.isThumb() || T.isAArch64()) {
+    // Previously 1 is used and meant for the backed to derive the function
+    // attribute form it. 2 now means function attributes already set for all
+    // functions in this module, so no need to propagate those from the module
+    // flag. Value is only used in case of LTO module merge because the backend
+    // will see all required function attribute set already. Value is used
+    // before modules got merged. Any posive value means the feature is active
+    // and required binary markings need to be emit accordingly.
     if (LangOpts.BranchTargetEnforcement)
       getModule().addModuleFlag(llvm::Module::Min, "branch-target-enforcement",
-                                1);
+                                2);
     if (LangOpts.BranchProtectionPAuthLR)
       getModule().addModuleFlag(llvm::Module::Min, "branch-protection-pauth-lr",
-                                1);
+                                2);
     if (LangOpts.GuardedControlStack)
-      getModule().addModuleFlag(llvm::Module::Min, "guarded-control-stack", 1);
+      getModule().addModuleFlag(llvm::Module::Min, "guarded-control-stack", 2);
     if (LangOpts.hasSignReturnAddress())
-      getModule().addModuleFlag(llvm::Module::Min, "sign-return-address", 1);
+      getModule().addModuleFlag(llvm::Module::Min, "sign-return-address", 2);
     if (LangOpts.isSignReturnAddressScopeAll())
       getModule().addModuleFlag(llvm::Module::Min, "sign-return-address-all",
-                                1);
+                                2);
     if (!LangOpts.isSignReturnAddressWithAKey())
       getModule().addModuleFlag(llvm::Module::Min,
-                                "sign-return-address-with-bkey", 1);
+                                "sign-return-address-with-bkey", 2);
   }
 
   if (CodeGenOpts.StackClashProtector)
diff --git a/clang/test/CodeGen/aarch64-sign-return-address.c b/clang/test/CodeGen/aarch64-sign-return-address.c
index 8bc54b1a56c38c..35c56889e07071 100644
--- a/clang/test/CodeGen/aarch64-sign-return-address.c
+++ b/clang/test/CodeGen/aarch64-sign-return-address.c
@@ -22,17 +22,17 @@
 // NONE-NOT:  !"branch-target-enforcement"
 // ALL-NOT:   !"branch-target-enforcement"
 // PART-NOT:  !"branch-target-enforcement"
-// BTE:       !{i32 8, !"branch-target-enforcement", i32 1}
+// BTE:       !{i32 8, !"branch-target-enforcement", i32 2}
 // B-KEY-NOT: !"branch-target-enforcement"
 
 // NONE-NOT:  !"sign-return-address"
-// ALL:   !{i32 8, !"sign-return-address", i32 1}
-// PART:  !{i32 8, !"sign-return-address", i32 1}
+// ALL:   !{i32 8, !"sign-return-address", i32 2}
+// PART:  !{i32 8, !"sign-return-address", i32 2}
 // BTE-NOT:   !"sign-return-address"
-// B-KEY: !{i32 8, !"sign-return-address", i32 1}
+// B-KEY: !{i32 8, !"sign-return-address", i32 2}
 
 // NONE-NOT:  !"sign-return-address-all"
-// ALL:   !{i32 8, !"sign-return-address-all", i32 1}
+// ALL:   !{i32 8, !"sign-return-address-all", i32 2}
 // PART-NOT:  !"sign-return-address-all"
 // BTE-NOT:   !"sign-return-address-all"
 // B-KEY-NOT: !"sign-return-address-all"
@@ -41,6 +41,6 @@
 // ALL-NOT:   !"sign-return-address-with-bkey"
 // PART-NOT:  !"sign-return-address-with-bkey"
 // BTE-NOT:   !"sign-return-address-with-bkey"
-// B-KEY: !{i32 8, !"sign-return-address-with-bkey", i32 1}
+// B-KEY: !{i32 8, !"sign-return-address-with-bkey", i32 2}
 
 void foo() {}
diff --git a/clang/test/CodeGen/arm-branch-protection-attr-2.c b/clang/test/CodeGen/arm-branch-protection-attr-2.c
index 1f3c00873043e8..741c0026c4d05c 100644
--- a/clang/test/CodeGen/arm-branch-protection-attr-2.c
+++ b/clang/test/CodeGen/arm-branch-protection-attr-2.c
@@ -18,16 +18,16 @@
 // NONE-NOT: !"branch-target-enforcement"
 // PART-NOT: !"branch-target-enforcement"
 // ALL-NOT:  !"branch-target-enforcement"
-// BTE:      !{i32 8, !"branch-target-enforcement", i32 1}
+// BTE:      !{i32 8, !"branch-target-enforcement", i32 2}
 
 // NONE-NOT: !"sign-return-address"
-// PART:     !{i32 8, !"sign-return-address", i32 1}
-// ALL:      !{i32 8, !"sign-return-address", i32 1}
+// PART:     !{i32 8, !"sign-return-address", i32 2}
+// ALL:      !{i32 8, !"sign-return-address", i32 2}
 // BTE-NOT:  !"sign-return-address"
 
 // NONE-NOT: !"sign-return-address-all", i32 0}
 // PART-NOT: !"sign-return-address-all", i32 0}
-// ALL:      !{i32 8, !"sign-return-address-all", i32 1}
+// ALL:      !{i32 8, !"sign-return-address-all", i32 2}
 // BTE-NOT:  !"sign-return-address-all", i32 0}
 
 void foo() {}
diff --git a/clang/test/Frontend/arm-ignore-branch-protection-option.c b/clang/test/Frontend/arm-ignore-branch-protection-option.c
index 99a2accef3ae2f..45bdb37f5ed1a3 100644
--- a/clang/test/Frontend/arm-ignore-branch-protection-option.c
+++ b/clang/test/Frontend/arm-ignore-branch-protection-option.c
@@ -15,4 +15,4 @@ __attribute__((target("arch=cortex-m0"))) void f() {}
 // CHECK-NOT:  attributes { {{.*}} "branch-target-enforcement"
 
 /// Check that there are branch protection module attributes despite the warning.
-// CHECK: !{i32 8, !"branch-target-enforcement", i32 1}
+// CHECK: !{i32 8, !"branch-target-enforcement", i32 2}
diff --git a/lld/test/ELF/lto/aarch64_inline.ll b/lld/test/ELF/lto/aarch64_inline.ll
new file mode 100644
index 00000000000000..67fa74e528dee9
--- /dev/null
+++ b/lld/test/ELF/lto/aarch64_inline.ll
@@ -0,0 +1,73 @@
+; REQUIRES: aarch64
+;; Test verifies inlining happens cross module when module flags are upgraded
+;; by the thin-lto linker/IRMover.
+;; Regression test for #82763
+
+; RUN: split-file %s %t
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -unified-lto %t/foo.s -o %t/foo.o
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -unified-lto %t/bar.s -o %t/bar.o
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -unified-lto %t/main.s -o %t/main.o
+; RUN: ld.lld -O2 --lto=thin --entry=main %t/main.o %t/foo.o %t/bar.o -o %t/exe
+; RUN: llvm-objdump -d %t/exe | FileCheck %s
+
+
+; CHECK-LABEL:  <main>:
+; CHECK-NEXT:         pacibsp
+; CHECK-NEXT:         mov     w0, #0x23
+; CHECK-NEXT:         autibsp
+; CHECK-NEXT:         ret
+
+;--- foo.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"
+
+define dso_local noundef i32 @foo() local_unnamed_addr #0 {
+entry:
+  ret i32 34
+}
+
+attributes #0 = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) }
+!llvm.module.flags = !{!0, !1, !2, !3 }
+!0 = !{i32 8, !"branch-target-enforcement", i32 1}
+!1 = !{i32 8, !"sign-return-address", i32 1}
+!2 = !{i32 8, !"sign-return-address-all", i32 1}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 1}
+
+;--- bar.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"
+
+define dso_local noundef i32 @bar() local_unnamed_addr #0 {
+entry:
+  ret i32 1
+}
+
+attributes #0 = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) "branch-target-enforcement" "sign-return-address"="all" "sign-return-address-key"="b_key" }
+!llvm.module.flags = !{!0, !1, !2, !3 }
+!0 = !{i32 8, !"branch-target-enforcement", i32 2}
+!1 = !{i32 8, !"sign-return-address", i32 2}
+!2 = !{i32 8, !"sign-return-address-all", i32 2}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 2}
+
+;--- main.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();
+declare i32 @bar();
+
+define i32 @main() #0 {
+entry:
+  %1 = call i32 @foo()
+  %2 = call i32 @bar()
+  %3 = add i32 %1, %2
+  ret i32 %3
+}
+
+attributes #0 = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) }
+
+!llvm.module.flags = !{!0, !1, !2, !3 }
+!0 = !{i32 8, !"branch-target-enforcement", i32 1}
+!1 = !{i32 8, !"sign-return-address", i32 1}
+!2 = !{i32 8, !"sign-return-address-all", i32 1}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 1}
diff --git a/llvm/include/llvm/IR/AutoUpgrade.h b/llvm/include/llvm/IR/AutoUpgrade.h
index 152f781ffa9b30..1ef32bcb121bec 100644
--- a/llvm/include/llvm/IR/AutoUpgrade.h
+++ b/llvm/include/llvm/IR/AutoUpgrade.h
@@ -88,6 +88,9 @@ namespace llvm {
   /// info. Return true if module is modified.
   bool UpgradeDebugInfo(Module &M);
 
+  /// Copies module attributes to the functions in the module.
+  void CopyModuleAttrToFunctions(Module &M);
+
   /// Check whether a string looks like an old loop attachment tag.
   inline bool mayBeOldLoopAttachmentTag(StringRef Name) {
     return Name.starts_with("llvm.vectorizer.");
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index f0be021668afa7..299e0d9f29953d 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -408,6 +408,7 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
 
   UpgradeModuleFlags(*M);
   UpgradeSectionAttributes(*M);
+  CopyModuleAttrToFunctions(*M);
 
   if (!Slots)
     return false;
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 3fc8141381c623..a07b4aa0b5cd5d 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -6927,6 +6927,8 @@ Error BitcodeReader::materializeModule() {
 
   UpgradeARCRuntime(*TheModule);
 
+  CopyModuleAttrToFunctions(*TheModule);
+
   return Error::success();
 }
 
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index a44f6af4162f03..8a671657c9c9ec 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -5250,6 +5250,106 @@ void llvm::UpgradeFunctionAttributes(Function &F) {
     Arg.removeAttrs(AttributeFuncs::typeIncompatible(Arg.getType()));
 }
 
+// Check if the module attribute is present and set to one.
+static bool isModuleAttributeOne(Module &M, const StringRef &ModAttr) {
+  const auto *Attr =
+      mdconst::extract_or_null<ConstantInt>(M.getModuleFlag(ModAttr));
+  return Attr && Attr->isOne();
+}
+
+// Check if the module attribute is present and set to two.
+static bool isModuleAttributeTwo(Module &M, const StringRef &ModAttr) {
+  const auto *Attr =
+      mdconst::extract_or_null<ConstantInt>(M.getModuleFlag(ModAttr));
+  return Attr && Attr->getZExtValue() == 2;
+}
+
+// Check if the function attribute is not present and set it.
+static void SetFunctionAttrIfNotSet(Function &F, StringRef FnAttrName,
+                                    StringRef Value) {
+  if (!F.hasFnAttribute(FnAttrName))
+    F.addFnAttr(FnAttrName, Value);
+}
+
+// Check if the function attribute is not present and set it if needed.
+// If the attribute is "false" then removes it.
+// If the attribute is "true" resets it to a valueless attribute.
+static void ConvertFunctionAttr(Function &F, bool Set, StringRef FnAttrName) {
+  if (!F.hasFnAttribute(FnAttrName)) {
+    if (Set)
+      F.addFnAttr(FnAttrName);
+  } else {
+    auto A = F.getFnAttribute(FnAttrName);
+    if ("false" == A.getValueAsString())
+      F.removeFnAttr(FnAttrName);
+    else if ("true" == A.getValueAsString()) {
+      F.removeFnAttr(FnAttrName);
+      F.addFnAttr(FnAttrName);
+    }
+  }
+}
+
+void llvm::CopyModuleAttrToFunctions(Module &M) {
+  Triple T(M.getTargetTriple());
+  if (!T.isThumb() && !T.isARM() && !T.isAArch64())
+    return;
+
+  if (isModuleAttributeTwo(M, "branch-target-enforcement"))
+    return;
+  if (isModuleAttributeTwo(M, "branch-protection-pauth-lr"))
+    return;
+  if (isModuleAttributeTwo(M, "guarded-control-stack"))
+    return;
+  if (isModuleAttributeTwo(M, "sign-return-address"))
+    return;
+
+  bool BTE = isModuleAttributeOne(M, "branch-target-enforcement");
+  bool BPPLR = isModuleAttributeOne(M, "branch-protection-pauth-lr");
+  bool GCS = isModuleAttributeOne(M, "guarded-control-stack");
+  bool SRA = isModuleAttributeOne(M, "sign-return-address");
+
+  StringRef SignTypeValue = "non-leaf";
+  if (SRA && isModuleAttributeOne(M, "sign-return-address-all"))
+    SignTypeValue = "all";
+
+  StringRef SignKeyValue = "a_key";
+  if (SRA && isModuleAttributeOne(M, "sign-return-address-with-bkey"))
+    SignKeyValue = "b_key";
+
+  for (Function &F : M.getFunctionList()) {
+    if (F.isDeclaration())
+      continue;
+
+    if (SRA) {
+      SetFunctionAttrIfNotSet(F, "sign-return-address", SignTypeValue);
+      SetFunctionAttrIfNotSet(F, "sign-return-address-key", SignKeyValue);
+    } else {
+      if (auto A = F.getFnAttribute("sign-return-address");
+          A.isValid() && "none" == A.getValueAsString()) {
+        F.removeFnAttr("sign-return-address");
+        F.removeFnAttr("sign-return-address-key");
+      }
+    }
+    ConvertFunctionAttr(F, BTE, "branch-target-enforcement");
+    ConvertFunctionAttr(F, BPPLR, "branch-protection-pauth-lr");
+    ConvertFunctionAttr(F, GCS, "guarded-control-stack");
+  }
+
+  if (BTE)
+    M.setModuleFlag(llvm::Module::Min, "branch-target-enforcement", 2);
+  if (BPPLR)
+    M.setModuleFlag(llvm::Module::Min, "branch-protection-pauth-lr", 2);
+  if (GCS)
+    M.setModuleFlag(llvm::Module::Min, "guarded-control-stack", 2);
+  if (SRA) {
+    M.setModuleFlag(llvm::Module::Min, "sign-return-address", 2);
+    if (isModuleAttributeOne(M, "sign-return-address-all"))
+      M.setModuleFlag(llvm::Module::Min, "sign-return-address-all", 2);
+    if (isModuleAttributeOne(M, "sign-return-address-with-bkey"))
+      M.setModuleFlag(llvm::Module::Min, "sign-return-address-with-bkey", 2);
+  }
+}
+
 static bool isOldLoopArgument(Metadata *MD) {
   auto *T = dyn_cast_or_null<MDTuple>(MD);
   if (!T)
diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index a7e6db82e5c23c..b019cd201313df 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -1623,6 +1623,11 @@ Error IRLinker::run() {
   // Loop over all of the linked values to compute type mappings.
   computeTypeMapping();
 
+  // 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.
+  CopyModuleAttrToFunctions(*SrcM);
+
   std::reverse(Worklist.begin(), Worklist.end());
   while (!Worklist.empty()) {
     GlobalValue *GV = Worklist.back();
@@ -1787,6 +1792,11 @@ IRMover::IRMover(Module &M) : Composite(M) {
   for (const auto *MD : StructTypes.getVisitedMetadata()) {
     SharedMDs[MD].reset(const_cast<MDNode *>(MD));
   }
+
+  // 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.
+  CopyModuleAttrToFunctions(M);
 }
 
 Error IRMover::move(std::unique_ptr<Module> Src,
diff --git a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
index 642739a29d6b06..fe9f207a53e436 100644
--- a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
+++ b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
@@ -793,7 +793,7 @@ void ARMAsmPrinter::emitAttributes() {
 
     auto *BTIValue = mdconst::extract_or_null<ConstantInt>(
         SourceModule->getModuleFlag("branch-target-enforcement"));
-    if (BTIValue && BTIValue->isOne()) {
+    if (BTIValue && !BTIValue->isZero()) {
       // If "+pacbti" is used as an architecture extension,
       // Tag_BTI_extension is emitted in
       // ARMTargetStreamer::emitTargetAttributes().
diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
index 633fcb3314c42f..943360f725ce1f 100644
--- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -1211,7 +1211,7 @@ bool LowerTypeTestsModule::hasBranchTargetEnforcement() {
     // the module flags.
     if (const auto *BTE = mdconst::extract_or_null<ConstantInt>(
           M.getModuleFlag("branch-target-enforcement")))
-      HasBranchTargetEnforcement = (BTE->getZExtValue() != 0);
+      HasBranchTargetEnforcement = !BTE->isZero();
     else
       HasBranchTargetEnforcement = 0;
   }
diff --git a/llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll b/llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll
index 6bfaf3bb520a0a..77c3424f4abf30 100644
--- a/llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll
+++ b/llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll
@@ -94,5 +94,5 @@ attributes #1 = { minsize nofree norecurse nounwind optsize "sign-return-address
 !llvm.module.flags = !{!0, !1, !2}
 
 !0 = !{i32 8, !"branch-target-enforcement", i32 0}
-!1 = !{i32 8, !"sign-return-address", i32 1}
+!1 = !{i32 8, !"sign-return-address", i32 2}
 !2 = !{i32 8, !"sign-return-address-all", i32 0}
diff --git a/llvm/test/LTO/AArch64/Inputs/foo.ll b/llvm/test/LTO/AArch64/Inputs/foo.ll
deleted file mode 100644
index 961b0d4e7997e1..00000000000000
--- a/llvm/test/LTO/AArch64/Inputs/foo.ll
+++ /dev/null
@@ -1,16 +0,0 @@
-target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
-target triple = "aarch64-unknown-linux-gnu"
-
-define dso_local i32 @foo() #0 {
-entry:
-  ret i32 42
-}
-
-attributes #0 = { noinline nounwind optnone uwtable }
-
-!llvm.module.flags = !{!0, !1, !2, !3}
-
-!0 = !{i32 8, !"branch-target-enforcement", i32 1}
-!1 = !{i32 8, !"sign-return-address", i32 1}
-!2 = !{i32 8, !"sign-return-address-all", i32 1}
-!3 = !{i32 8, !"sign-return-address-with-bkey", i32 1}
diff --git a/llvm/test/LTO/AArch64/TestInputs/bar.ll b/llvm/test/LTO/AArch64/TestInputs/bar.ll
new file mode 100644
index 00000000000000..e8a92767c8d7a3
--- /dev/null
+++ b/llvm/test/LTO/AArch64/TestInputs/bar.ll
@@ -0,0 +1,35 @@
+; This file contains the new semantic of the branch-target-enforcement, sign-return-address.
+; Used for test mixing a mixed link case and also verify the import too in llc.
+
+; RUN: llc %s -o - | FileCheck %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"
+
+define dso_local void @bar() #0 {
+entry:
+  ret void
+}
+; CHECK-LABEL: bar:
+; CHECK-NOT:       hint
+; CHECK-NOT:       bti
+; CHECK:           ret
+
+define dso_local void @baz() #1 {
+entry:
+  ret void
+}
+
+; CHECK-LABEL: baz:
+; CHECK:           hint
+; CHECK:           ret
+
+attributes #0 = { noinline nounwind optnone uwtable }
+attributes #1 = { noinline nounwind optnone uwtable "branch-target-enforcement" }
+
+!llvm.module.flags = !{!0, !1, !2, !3}
+
+!0 = !{i32 8, !"branch-target-enforcement", i32 2}
+!1 = !{i32 8, !"sign-return-address", i32 2}
+!2 = !{i32 8, !"sign-return-address-all", i32 2}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 2}
diff --git a/llvm/test/LTO/AArch64/TestInputs/foo.ll b/llvm/test/LTO/AArch64/TestInputs/foo.ll
new file mode 100644
index 00000000000000..7a4cb2ce5ade78
--- /dev/null
+++ b/llvm/test/LTO/AArch64/TestInputs/foo.ll
@@ -0,0 +1,38 @@
+; This file contains the previous semantic of the branch-target-enforcement, sign-return-address.
+; Used for test mixing a mixed link case and also verify the import too in llc.
+
+; RUN: llc %s -o - | FileCheck %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"
+
+define i32 @foo() #0 {
+entry:
+  ret i32 42
+}
+
+; CHECK-LABEL: foo:
+; CHECK:           hint    #27
+; CHECK:           mov
+; CHECK:           hint    #31
+; CHECK:           ret
+
+define i32 @fiz() #1 {
+entry:
+  ret i32 43
+}
+
+; CHECK-LABEL: fiz:
+; CHECK-NOT:       hint
+; CHECK-NOT:       bti
+; CHECK:           ret
+
+attributes #0 = { noinline nounwind optnone uwtable }
+attributes #1 = { noinline nounwind optnone uwtable "branch-target-enforcement"="false" "sign-return-address"="none" }
+
+!llvm.module.flags = !{!0, !1, !2, !3}
+
+!0 = !{i32 8, !"branch-target-enforcement", i32 1}
+!1 = !{i32 8, !"sign-return-address", i32 1}
+!2 = !{i32 8, !"sign-return-address-all", i32 1}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 1}
diff --git a/llvm/test/LTO/AArch64/TestInputs/old.ll b/llvm/test/LTO/AArch64/TestInputs/old.ll
new file mode 100644
index 00000000000000..119ea6fabbd70a
--- /d...
[truncated]

llvmbot · 2024-03-21T22:39:16Z

@llvm/pr-subscribers-llvm-ir

Author: Daniel Kiss (DanielKristofKiss)

Changes

Module flag is used to indicate the feature to be propagated to the function. As now the frontend emits all attributes accordingly let's help the auto upgrade to only do work when old and new bitcodes are merged.

Depends on #82819 and #86031

Patch is 26.38 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86212.diff

20 Files Affected:

(modified) clang/lib/CodeGen/CodeGenModule.cpp (+13-6)
(modified) clang/test/CodeGen/aarch64-sign-return-address.c (+6-6)
(modified) clang/test/CodeGen/arm-branch-protection-attr-2.c (+4-4)
(modified) clang/test/Frontend/arm-ignore-branch-protection-option.c (+1-1)
(added) lld/test/ELF/lto/aarch64_inline.ll (+73)
(modified) llvm/include/llvm/IR/AutoUpgrade.h (+3)
(modified) llvm/lib/AsmParser/LLParser.cpp (+1)
(modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+2)
(modified) llvm/lib/IR/AutoUpgrade.cpp (+100)
(modified) llvm/lib/Linker/IRMover.cpp (+10)
(modified) llvm/lib/Target/ARM/ARMAsmPrinter.cpp (+1-1)
(modified) llvm/lib/Transforms/IPO/LowerTypeTests.cpp (+1-1)
(modified) llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll (+1-1)
(removed) llvm/test/LTO/AArch64/Inputs/foo.ll (-16)
(added) llvm/test/LTO/AArch64/TestInputs/bar.ll (+35)
(added) llvm/test/LTO/AArch64/TestInputs/foo.ll (+38)
(added) llvm/test/LTO/AArch64/TestInputs/old.ll (+46)
(modified) llvm/test/LTO/AArch64/link-branch-target-enforcement.ll (+4-1)
(added) llvm/test/LTO/AArch64/link-sign-return-address.ll (+102)
(modified) llvm/test/Linker/link-arm-and-thumb.ll (+3-3)

diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index cb153066b28dd1..1acc0510256268 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1173,22 +1173,29 @@ void CodeGenModule::Release() {
                               "tag-stack-memory-buildattr", 1);
 
   if (T.isARM() || T.isThumb() || T.isAArch64()) {
+    // Previously 1 is used and meant for the backed to derive the function
+    // attribute form it. 2 now means function attributes already set for all
+    // functions in this module, so no need to propagate those from the module
+    // flag. Value is only used in case of LTO module merge because the backend
+    // will see all required function attribute set already. Value is used
+    // before modules got merged. Any posive value means the feature is active
+    // and required binary markings need to be emit accordingly.
     if (LangOpts.BranchTargetEnforcement)
       getModule().addModuleFlag(llvm::Module::Min, "branch-target-enforcement",
-                                1);
+                                2);
     if (LangOpts.BranchProtectionPAuthLR)
       getModule().addModuleFlag(llvm::Module::Min, "branch-protection-pauth-lr",
-                                1);
+                                2);
     if (LangOpts.GuardedControlStack)
-      getModule().addModuleFlag(llvm::Module::Min, "guarded-control-stack", 1);
+      getModule().addModuleFlag(llvm::Module::Min, "guarded-control-stack", 2);
     if (LangOpts.hasSignReturnAddress())
-      getModule().addModuleFlag(llvm::Module::Min, "sign-return-address", 1);
+      getModule().addModuleFlag(llvm::Module::Min, "sign-return-address", 2);
     if (LangOpts.isSignReturnAddressScopeAll())
       getModule().addModuleFlag(llvm::Module::Min, "sign-return-address-all",
-                                1);
+                                2);
     if (!LangOpts.isSignReturnAddressWithAKey())
       getModule().addModuleFlag(llvm::Module::Min,
-                                "sign-return-address-with-bkey", 1);
+                                "sign-return-address-with-bkey", 2);
   }
 
   if (CodeGenOpts.StackClashProtector)
diff --git a/clang/test/CodeGen/aarch64-sign-return-address.c b/clang/test/CodeGen/aarch64-sign-return-address.c
index 8bc54b1a56c38c..35c56889e07071 100644
--- a/clang/test/CodeGen/aarch64-sign-return-address.c
+++ b/clang/test/CodeGen/aarch64-sign-return-address.c
@@ -22,17 +22,17 @@
 // NONE-NOT:  !"branch-target-enforcement"
 // ALL-NOT:   !"branch-target-enforcement"
 // PART-NOT:  !"branch-target-enforcement"
-// BTE:       !{i32 8, !"branch-target-enforcement", i32 1}
+// BTE:       !{i32 8, !"branch-target-enforcement", i32 2}
 // B-KEY-NOT: !"branch-target-enforcement"
 
 // NONE-NOT:  !"sign-return-address"
-// ALL:   !{i32 8, !"sign-return-address", i32 1}
-// PART:  !{i32 8, !"sign-return-address", i32 1}
+// ALL:   !{i32 8, !"sign-return-address", i32 2}
+// PART:  !{i32 8, !"sign-return-address", i32 2}
 // BTE-NOT:   !"sign-return-address"
-// B-KEY: !{i32 8, !"sign-return-address", i32 1}
+// B-KEY: !{i32 8, !"sign-return-address", i32 2}
 
 // NONE-NOT:  !"sign-return-address-all"
-// ALL:   !{i32 8, !"sign-return-address-all", i32 1}
+// ALL:   !{i32 8, !"sign-return-address-all", i32 2}
 // PART-NOT:  !"sign-return-address-all"
 // BTE-NOT:   !"sign-return-address-all"
 // B-KEY-NOT: !"sign-return-address-all"
@@ -41,6 +41,6 @@
 // ALL-NOT:   !"sign-return-address-with-bkey"
 // PART-NOT:  !"sign-return-address-with-bkey"
 // BTE-NOT:   !"sign-return-address-with-bkey"
-// B-KEY: !{i32 8, !"sign-return-address-with-bkey", i32 1}
+// B-KEY: !{i32 8, !"sign-return-address-with-bkey", i32 2}
 
 void foo() {}
diff --git a/clang/test/CodeGen/arm-branch-protection-attr-2.c b/clang/test/CodeGen/arm-branch-protection-attr-2.c
index 1f3c00873043e8..741c0026c4d05c 100644
--- a/clang/test/CodeGen/arm-branch-protection-attr-2.c
+++ b/clang/test/CodeGen/arm-branch-protection-attr-2.c
@@ -18,16 +18,16 @@
 // NONE-NOT: !"branch-target-enforcement"
 // PART-NOT: !"branch-target-enforcement"
 // ALL-NOT:  !"branch-target-enforcement"
-// BTE:      !{i32 8, !"branch-target-enforcement", i32 1}
+// BTE:      !{i32 8, !"branch-target-enforcement", i32 2}
 
 // NONE-NOT: !"sign-return-address"
-// PART:     !{i32 8, !"sign-return-address", i32 1}
-// ALL:      !{i32 8, !"sign-return-address", i32 1}
+// PART:     !{i32 8, !"sign-return-address", i32 2}
+// ALL:      !{i32 8, !"sign-return-address", i32 2}
 // BTE-NOT:  !"sign-return-address"
 
 // NONE-NOT: !"sign-return-address-all", i32 0}
 // PART-NOT: !"sign-return-address-all", i32 0}
-// ALL:      !{i32 8, !"sign-return-address-all", i32 1}
+// ALL:      !{i32 8, !"sign-return-address-all", i32 2}
 // BTE-NOT:  !"sign-return-address-all", i32 0}
 
 void foo() {}
diff --git a/clang/test/Frontend/arm-ignore-branch-protection-option.c b/clang/test/Frontend/arm-ignore-branch-protection-option.c
index 99a2accef3ae2f..45bdb37f5ed1a3 100644
--- a/clang/test/Frontend/arm-ignore-branch-protection-option.c
+++ b/clang/test/Frontend/arm-ignore-branch-protection-option.c
@@ -15,4 +15,4 @@ __attribute__((target("arch=cortex-m0"))) void f() {}
 // CHECK-NOT:  attributes { {{.*}} "branch-target-enforcement"
 
 /// Check that there are branch protection module attributes despite the warning.
-// CHECK: !{i32 8, !"branch-target-enforcement", i32 1}
+// CHECK: !{i32 8, !"branch-target-enforcement", i32 2}
diff --git a/lld/test/ELF/lto/aarch64_inline.ll b/lld/test/ELF/lto/aarch64_inline.ll
new file mode 100644
index 00000000000000..67fa74e528dee9
--- /dev/null
+++ b/lld/test/ELF/lto/aarch64_inline.ll
@@ -0,0 +1,73 @@
+; REQUIRES: aarch64
+;; Test verifies inlining happens cross module when module flags are upgraded
+;; by the thin-lto linker/IRMover.
+;; Regression test for #82763
+
+; RUN: split-file %s %t
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -unified-lto %t/foo.s -o %t/foo.o
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -unified-lto %t/bar.s -o %t/bar.o
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -unified-lto %t/main.s -o %t/main.o
+; RUN: ld.lld -O2 --lto=thin --entry=main %t/main.o %t/foo.o %t/bar.o -o %t/exe
+; RUN: llvm-objdump -d %t/exe | FileCheck %s
+
+
+; CHECK-LABEL:  <main>:
+; CHECK-NEXT:         pacibsp
+; CHECK-NEXT:         mov     w0, #0x23
+; CHECK-NEXT:         autibsp
+; CHECK-NEXT:         ret
+
+;--- foo.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"
+
+define dso_local noundef i32 @foo() local_unnamed_addr #0 {
+entry:
+  ret i32 34
+}
+
+attributes #0 = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) }
+!llvm.module.flags = !{!0, !1, !2, !3 }
+!0 = !{i32 8, !"branch-target-enforcement", i32 1}
+!1 = !{i32 8, !"sign-return-address", i32 1}
+!2 = !{i32 8, !"sign-return-address-all", i32 1}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 1}
+
+;--- bar.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"
+
+define dso_local noundef i32 @bar() local_unnamed_addr #0 {
+entry:
+  ret i32 1
+}
+
+attributes #0 = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) "branch-target-enforcement" "sign-return-address"="all" "sign-return-address-key"="b_key" }
+!llvm.module.flags = !{!0, !1, !2, !3 }
+!0 = !{i32 8, !"branch-target-enforcement", i32 2}
+!1 = !{i32 8, !"sign-return-address", i32 2}
+!2 = !{i32 8, !"sign-return-address-all", i32 2}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 2}
+
+;--- main.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();
+declare i32 @bar();
+
+define i32 @main() #0 {
+entry:
+  %1 = call i32 @foo()
+  %2 = call i32 @bar()
+  %3 = add i32 %1, %2
+  ret i32 %3
+}
+
+attributes #0 = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) }
+
+!llvm.module.flags = !{!0, !1, !2, !3 }
+!0 = !{i32 8, !"branch-target-enforcement", i32 1}
+!1 = !{i32 8, !"sign-return-address", i32 1}
+!2 = !{i32 8, !"sign-return-address-all", i32 1}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 1}
diff --git a/llvm/include/llvm/IR/AutoUpgrade.h b/llvm/include/llvm/IR/AutoUpgrade.h
index 152f781ffa9b30..1ef32bcb121bec 100644
--- a/llvm/include/llvm/IR/AutoUpgrade.h
+++ b/llvm/include/llvm/IR/AutoUpgrade.h
@@ -88,6 +88,9 @@ namespace llvm {
   /// info. Return true if module is modified.
   bool UpgradeDebugInfo(Module &M);
 
+  /// Copies module attributes to the functions in the module.
+  void CopyModuleAttrToFunctions(Module &M);
+
   /// Check whether a string looks like an old loop attachment tag.
   inline bool mayBeOldLoopAttachmentTag(StringRef Name) {
     return Name.starts_with("llvm.vectorizer.");
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index f0be021668afa7..299e0d9f29953d 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -408,6 +408,7 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
 
   UpgradeModuleFlags(*M);
   UpgradeSectionAttributes(*M);
+  CopyModuleAttrToFunctions(*M);
 
   if (!Slots)
     return false;
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 3fc8141381c623..a07b4aa0b5cd5d 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -6927,6 +6927,8 @@ Error BitcodeReader::materializeModule() {
 
   UpgradeARCRuntime(*TheModule);
 
+  CopyModuleAttrToFunctions(*TheModule);
+
   return Error::success();
 }
 
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index a44f6af4162f03..8a671657c9c9ec 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -5250,6 +5250,106 @@ void llvm::UpgradeFunctionAttributes(Function &F) {
     Arg.removeAttrs(AttributeFuncs::typeIncompatible(Arg.getType()));
 }
 
+// Check if the module attribute is present and set to one.
+static bool isModuleAttributeOne(Module &M, const StringRef &ModAttr) {
+  const auto *Attr =
+      mdconst::extract_or_null<ConstantInt>(M.getModuleFlag(ModAttr));
+  return Attr && Attr->isOne();
+}
+
+// Check if the module attribute is present and set to two.
+static bool isModuleAttributeTwo(Module &M, const StringRef &ModAttr) {
+  const auto *Attr =
+      mdconst::extract_or_null<ConstantInt>(M.getModuleFlag(ModAttr));
+  return Attr && Attr->getZExtValue() == 2;
+}
+
+// Check if the function attribute is not present and set it.
+static void SetFunctionAttrIfNotSet(Function &F, StringRef FnAttrName,
+                                    StringRef Value) {
+  if (!F.hasFnAttribute(FnAttrName))
+    F.addFnAttr(FnAttrName, Value);
+}
+
+// Check if the function attribute is not present and set it if needed.
+// If the attribute is "false" then removes it.
+// If the attribute is "true" resets it to a valueless attribute.
+static void ConvertFunctionAttr(Function &F, bool Set, StringRef FnAttrName) {
+  if (!F.hasFnAttribute(FnAttrName)) {
+    if (Set)
+      F.addFnAttr(FnAttrName);
+  } else {
+    auto A = F.getFnAttribute(FnAttrName);
+    if ("false" == A.getValueAsString())
+      F.removeFnAttr(FnAttrName);
+    else if ("true" == A.getValueAsString()) {
+      F.removeFnAttr(FnAttrName);
+      F.addFnAttr(FnAttrName);
+    }
+  }
+}
+
+void llvm::CopyModuleAttrToFunctions(Module &M) {
+  Triple T(M.getTargetTriple());
+  if (!T.isThumb() && !T.isARM() && !T.isAArch64())
+    return;
+
+  if (isModuleAttributeTwo(M, "branch-target-enforcement"))
+    return;
+  if (isModuleAttributeTwo(M, "branch-protection-pauth-lr"))
+    return;
+  if (isModuleAttributeTwo(M, "guarded-control-stack"))
+    return;
+  if (isModuleAttributeTwo(M, "sign-return-address"))
+    return;
+
+  bool BTE = isModuleAttributeOne(M, "branch-target-enforcement");
+  bool BPPLR = isModuleAttributeOne(M, "branch-protection-pauth-lr");
+  bool GCS = isModuleAttributeOne(M, "guarded-control-stack");
+  bool SRA = isModuleAttributeOne(M, "sign-return-address");
+
+  StringRef SignTypeValue = "non-leaf";
+  if (SRA && isModuleAttributeOne(M, "sign-return-address-all"))
+    SignTypeValue = "all";
+
+  StringRef SignKeyValue = "a_key";
+  if (SRA && isModuleAttributeOne(M, "sign-return-address-with-bkey"))
+    SignKeyValue = "b_key";
+
+  for (Function &F : M.getFunctionList()) {
+    if (F.isDeclaration())
+      continue;
+
+    if (SRA) {
+      SetFunctionAttrIfNotSet(F, "sign-return-address", SignTypeValue);
+      SetFunctionAttrIfNotSet(F, "sign-return-address-key", SignKeyValue);
+    } else {
+      if (auto A = F.getFnAttribute("sign-return-address");
+          A.isValid() && "none" == A.getValueAsString()) {
+        F.removeFnAttr("sign-return-address");
+        F.removeFnAttr("sign-return-address-key");
+      }
+    }
+    ConvertFunctionAttr(F, BTE, "branch-target-enforcement");
+    ConvertFunctionAttr(F, BPPLR, "branch-protection-pauth-lr");
+    ConvertFunctionAttr(F, GCS, "guarded-control-stack");
+  }
+
+  if (BTE)
+    M.setModuleFlag(llvm::Module::Min, "branch-target-enforcement", 2);
+  if (BPPLR)
+    M.setModuleFlag(llvm::Module::Min, "branch-protection-pauth-lr", 2);
+  if (GCS)
+    M.setModuleFlag(llvm::Module::Min, "guarded-control-stack", 2);
+  if (SRA) {
+    M.setModuleFlag(llvm::Module::Min, "sign-return-address", 2);
+    if (isModuleAttributeOne(M, "sign-return-address-all"))
+      M.setModuleFlag(llvm::Module::Min, "sign-return-address-all", 2);
+    if (isModuleAttributeOne(M, "sign-return-address-with-bkey"))
+      M.setModuleFlag(llvm::Module::Min, "sign-return-address-with-bkey", 2);
+  }
+}
+
 static bool isOldLoopArgument(Metadata *MD) {
   auto *T = dyn_cast_or_null<MDTuple>(MD);
   if (!T)
diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index a7e6db82e5c23c..b019cd201313df 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -1623,6 +1623,11 @@ Error IRLinker::run() {
   // Loop over all of the linked values to compute type mappings.
   computeTypeMapping();
 
+  // 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.
+  CopyModuleAttrToFunctions(*SrcM);
+
   std::reverse(Worklist.begin(), Worklist.end());
   while (!Worklist.empty()) {
     GlobalValue *GV = Worklist.back();
@@ -1787,6 +1792,11 @@ IRMover::IRMover(Module &M) : Composite(M) {
   for (const auto *MD : StructTypes.getVisitedMetadata()) {
     SharedMDs[MD].reset(const_cast<MDNode *>(MD));
   }
+
+  // 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.
+  CopyModuleAttrToFunctions(M);
 }
 
 Error IRMover::move(std::unique_ptr<Module> Src,
diff --git a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
index 642739a29d6b06..fe9f207a53e436 100644
--- a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
+++ b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
@@ -793,7 +793,7 @@ void ARMAsmPrinter::emitAttributes() {
 
     auto *BTIValue = mdconst::extract_or_null<ConstantInt>(
         SourceModule->getModuleFlag("branch-target-enforcement"));
-    if (BTIValue && BTIValue->isOne()) {
+    if (BTIValue && !BTIValue->isZero()) {
       // If "+pacbti" is used as an architecture extension,
       // Tag_BTI_extension is emitted in
       // ARMTargetStreamer::emitTargetAttributes().
diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
index 633fcb3314c42f..943360f725ce1f 100644
--- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -1211,7 +1211,7 @@ bool LowerTypeTestsModule::hasBranchTargetEnforcement() {
     // the module flags.
     if (const auto *BTE = mdconst::extract_or_null<ConstantInt>(
           M.getModuleFlag("branch-target-enforcement")))
-      HasBranchTargetEnforcement = (BTE->getZExtValue() != 0);
+      HasBranchTargetEnforcement = !BTE->isZero();
     else
       HasBranchTargetEnforcement = 0;
   }
diff --git a/llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll b/llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll
index 6bfaf3bb520a0a..77c3424f4abf30 100644
--- a/llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll
+++ b/llvm/test/CodeGen/Thumb2/pacbti-m-outliner-5.ll
@@ -94,5 +94,5 @@ attributes #1 = { minsize nofree norecurse nounwind optsize "sign-return-address
 !llvm.module.flags = !{!0, !1, !2}
 
 !0 = !{i32 8, !"branch-target-enforcement", i32 0}
-!1 = !{i32 8, !"sign-return-address", i32 1}
+!1 = !{i32 8, !"sign-return-address", i32 2}
 !2 = !{i32 8, !"sign-return-address-all", i32 0}
diff --git a/llvm/test/LTO/AArch64/Inputs/foo.ll b/llvm/test/LTO/AArch64/Inputs/foo.ll
deleted file mode 100644
index 961b0d4e7997e1..00000000000000
--- a/llvm/test/LTO/AArch64/Inputs/foo.ll
+++ /dev/null
@@ -1,16 +0,0 @@
-target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
-target triple = "aarch64-unknown-linux-gnu"
-
-define dso_local i32 @foo() #0 {
-entry:
-  ret i32 42
-}
-
-attributes #0 = { noinline nounwind optnone uwtable }
-
-!llvm.module.flags = !{!0, !1, !2, !3}
-
-!0 = !{i32 8, !"branch-target-enforcement", i32 1}
-!1 = !{i32 8, !"sign-return-address", i32 1}
-!2 = !{i32 8, !"sign-return-address-all", i32 1}
-!3 = !{i32 8, !"sign-return-address-with-bkey", i32 1}
diff --git a/llvm/test/LTO/AArch64/TestInputs/bar.ll b/llvm/test/LTO/AArch64/TestInputs/bar.ll
new file mode 100644
index 00000000000000..e8a92767c8d7a3
--- /dev/null
+++ b/llvm/test/LTO/AArch64/TestInputs/bar.ll
@@ -0,0 +1,35 @@
+; This file contains the new semantic of the branch-target-enforcement, sign-return-address.
+; Used for test mixing a mixed link case and also verify the import too in llc.
+
+; RUN: llc %s -o - | FileCheck %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"
+
+define dso_local void @bar() #0 {
+entry:
+  ret void
+}
+; CHECK-LABEL: bar:
+; CHECK-NOT:       hint
+; CHECK-NOT:       bti
+; CHECK:           ret
+
+define dso_local void @baz() #1 {
+entry:
+  ret void
+}
+
+; CHECK-LABEL: baz:
+; CHECK:           hint
+; CHECK:           ret
+
+attributes #0 = { noinline nounwind optnone uwtable }
+attributes #1 = { noinline nounwind optnone uwtable "branch-target-enforcement" }
+
+!llvm.module.flags = !{!0, !1, !2, !3}
+
+!0 = !{i32 8, !"branch-target-enforcement", i32 2}
+!1 = !{i32 8, !"sign-return-address", i32 2}
+!2 = !{i32 8, !"sign-return-address-all", i32 2}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 2}
diff --git a/llvm/test/LTO/AArch64/TestInputs/foo.ll b/llvm/test/LTO/AArch64/TestInputs/foo.ll
new file mode 100644
index 00000000000000..7a4cb2ce5ade78
--- /dev/null
+++ b/llvm/test/LTO/AArch64/TestInputs/foo.ll
@@ -0,0 +1,38 @@
+; This file contains the previous semantic of the branch-target-enforcement, sign-return-address.
+; Used for test mixing a mixed link case and also verify the import too in llc.
+
+; RUN: llc %s -o - | FileCheck %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"
+
+define i32 @foo() #0 {
+entry:
+  ret i32 42
+}
+
+; CHECK-LABEL: foo:
+; CHECK:           hint    #27
+; CHECK:           mov
+; CHECK:           hint    #31
+; CHECK:           ret
+
+define i32 @fiz() #1 {
+entry:
+  ret i32 43
+}
+
+; CHECK-LABEL: fiz:
+; CHECK-NOT:       hint
+; CHECK-NOT:       bti
+; CHECK:           ret
+
+attributes #0 = { noinline nounwind optnone uwtable }
+attributes #1 = { noinline nounwind optnone uwtable "branch-target-enforcement"="false" "sign-return-address"="none" }
+
+!llvm.module.flags = !{!0, !1, !2, !3}
+
+!0 = !{i32 8, !"branch-target-enforcement", i32 1}
+!1 = !{i32 8, !"sign-return-address", i32 1}
+!2 = !{i32 8, !"sign-return-address-all", i32 1}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 1}
diff --git a/llvm/test/LTO/AArch64/TestInputs/old.ll b/llvm/test/LTO/AArch64/TestInputs/old.ll
new file mode 100644
index 00000000000000..119ea6fabbd70a
--- /d...
[truncated]

Module flag is used to indicate the feature to be propagated to the function. As now the frontend emits all attributes accoringly let's help the automerger to only do work when old and new bitcodes are merged. Autoupgrade function attributes from Module attributes when needed.

llvm/include/llvm/IR/AutoUpgrade.h

llvm/lib/IR/AutoUpgrade.cpp

llvm/test/ThinLTO/AArch64/aarch64_inline.ll

llvm/test/LTO/AArch64/TestInputs/bar.ll

kovdan01

LGTM in terms of addressing my previous comments, and I also do not have new ones. But I'm not deeply into the context of the patch, so I'd prefer to wait for other reviews on this.

asavonic

LGTM overall. It is good that we address uncertainty of what these module flags actually mean. They went from "apply to all functions" to "only used to set an ELF flag", and it was impossible to tell which one it is.

asavonic · 2025-09-20T09:44:50Z

llvm/lib/IR/AutoUpgrade.cpp

+  if (!T.isThumb() && !T.isARM() && !T.isAArch64())
+    return;
+
+  if (isModuleAttributeTwo(M, "branch-target-enforcement"))


isModuleAttributeTwo and isModuleAttributeOne call Module::getModuleFlag function, which is a loop for each flag. This may look and perform better if we traverse flags only once, extract them as integers, and then do all the processing, like we do in UpgradeModuleFlags.

Sure, will be done.

asavonic · 2025-09-20T09:59:35Z

llvm/lib/Linker/IRMover.cpp

+  // 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.
+  CopyModuleAttrToFunctions(M);


Is the module here different from the module that we upgrade in IRLinker::run?

Yes, this "M" module here will not pop up the run as SrcM later.

asavonic · 2025-09-20T10:06:08Z

llvm/lib/Bitcode/Reader/BitcodeReader.cpp


  UpgradeARCRuntime(*TheModule);

+  CopyModuleAttrToFunctions(*TheModule);


I wonder if we need to do this here, or in BitcodeReader::globalCleanup, where we upgrade function attributes.

I quickly tried it but tests failed, didn't debug. why.

asavonic · 2025-10-10T03:20:53Z

@DanielKristofKiss, thanks a lot for updating the patch!
I see some cases where PAC-RET flags are not upgraded well, and they might be related to the points we discussed above.
I will post an update later and provide a reproducer.

asavonic · 2025-10-14T12:42:43Z

@DanielKristofKiss, thanks a lot for updating the patch! I see some cases where PAC-RET flags are not upgraded well, and they might be related to the points we discussed above. I will post an update later and provide a reproducer.

I have confirmed that the root cause of the issues I see is not what we discussed above. It only occurs for some cases of ThinLTO:

a module has no functions when CopyModuleAttrToFunctions is called the first time
we set module flags to 2, because there are no functions to upgrade
some functions are added later, CopyModuleAttrToFunctions is called again, but it does nothing because module flags are already set to 2.

I think it is an edge case. I can send a separate patch to not gate this one.

asavonic

LGTM

llvm/lib/IR/AutoUpgrade.cpp

DanielKristofKiss · 2025-10-14T18:42:19Z

@DanielKristofKiss, thanks a lot for updating the patch! I see some cases where PAC-RET flags are not upgraded well, and they might be related to the points we discussed above. I will post an update later and provide a reproducer.

I have confirmed that the root cause of the issues I see is not what we discussed above. It only occurs for some cases of ThinLTO:

a module has no functions when CopyModuleAttrToFunctions is called the first time

we set module flags to 2, because there are no functions to upgrade

some functions are added later, CopyModuleAttrToFunctions is called again, but it does nothing because module flags are already set to 2.

I think it is an edge case. I can send a separate patch to not gate this one.

Thanks for the feedback, if you have a reproduced happy to take a look.

DanielKristofKiss · 2025-10-21T20:29:05Z

@MaskRay @asavonic are you happy if this is landed?

asavonic · 2025-10-22T08:40:09Z

@MaskRay @asavonic are you happy if this is landed?

Thanks Daniel! Turned out that issues that we discussed above were caused by setModuleFlags. Patch #164580 should fix them.

It's standard in LLVM to have test inputs be in "Inputs". See https://llvm.org/docs/TestingGuide.html#extra-files. Fixes internal issue with #86212.

`Module::setModuleFlag` is supposed to change a single module. However, when an `MDNode` has the same value in more than one module in the same `LLVMContext`, such `MDNode` is shared (uniqued) across all of them. Therefore `MDNode::replaceOperandWith` changes all modules that share the same `MDNode`. This used to cause problems for #86212, where a module is marked as "upgraded" via a module flag. When this flag is shared across multiple modules, all of them are marked, yet some may not have been processed at all. After the patch we now construct a new `MDNode` and replace the old one.

Module flag is used to indicate the feature to be propagated to the function. As now the frontend emits all attributes accordingly let's help the auto upgrade to only do work when old and new bitcodes are merged. Depends on llvm#82819 and llvm#86031

It's standard in LLVM to have test inputs be in "Inputs". See https://llvm.org/docs/TestingGuide.html#extra-files. Fixes internal issue with llvm#86212.

`Module::setModuleFlag` is supposed to change a single module. However, when an `MDNode` has the same value in more than one module in the same `LLVMContext`, such `MDNode` is shared (uniqued) across all of them. Therefore `MDNode::replaceOperandWith` changes all modules that share the same `MDNode`. This used to cause problems for llvm#86212, where a module is marked as "upgraded" via a module flag. When this flag is shared across multiple modules, all of them are marked, yet some may not have been processed at all. After the patch we now construct a new `MDNode` and replace the old one.

Module flag is used to indicate the feature to be propagated to the function. As now the frontend emits all attributes accordingly let's help the auto upgrade to only do work when old and new bitcodes are merged. Depends on llvm#82819 and llvm#86031

It's standard in LLVM to have test inputs be in "Inputs". See https://llvm.org/docs/TestingGuide.html#extra-files. Fixes internal issue with llvm#86212.

`Module::setModuleFlag` is supposed to change a single module. However, when an `MDNode` has the same value in more than one module in the same `LLVMContext`, such `MDNode` is shared (uniqued) across all of them. Therefore `MDNode::replaceOperandWith` changes all modules that share the same `MDNode`. This used to cause problems for llvm#86212, where a module is marked as "upgraded" via a module flag. When this flag is shared across multiple modules, all of them are marked, yet some may not have been processed at all. After the patch we now construct a new `MDNode` and replace the old one.

Module flag is used to indicate the feature to be propagated to the function. As now the frontend emits all attributes accordingly let's help the auto upgrade to only do work when old and new bitcodes are merged. Depends on llvm#82819 and llvm#86031

It's standard in LLVM to have test inputs be in "Inputs". See https://llvm.org/docs/TestingGuide.html#extra-files. Fixes internal issue with llvm#86212.

`Module::setModuleFlag` is supposed to change a single module. However, when an `MDNode` has the same value in more than one module in the same `LLVMContext`, such `MDNode` is shared (uniqued) across all of them. Therefore `MDNode::replaceOperandWith` changes all modules that share the same `MDNode`. This used to cause problems for llvm#86212, where a module is marked as "upgraded" via a module flag. When this flag is shared across multiple modules, all of them are marked, yet some may not have been processed at all. After the patch we now construct a new `MDNode` and replace the old one.

DanielKristofKiss requested review from MaskRay, nickdesaulniers, statham-arm and tmatheson-arm March 21, 2024 22:38

llvmbot added clang Clang issues not falling into any other category lld backend:ARM clang:codegen IR generation bugs: mangling, exceptions, etc. lld:ELF LTO Link time optimization (regular/full LTO or ThinLTO) llvm:ir llvm:transforms labels Mar 21, 2024

DanielKristofKiss mentioned this pull request Mar 21, 2024

[ARM][AArch64] Change module flags values. #84804

Closed

DanielKristofKiss requested a review from dhoekwater March 21, 2024 22:40

This was referenced Mar 21, 2024

[Clang][ARM][AArch64] Alway emit protection attributes for functions. #82819

Merged

[LLVM] Refactor Autoupgrade function attributes from Module attributes. #84494

Closed

DanielKristofKiss force-pushed the ModuleFlagUpdate branch from cebd8b3 to f2f3356 Compare May 20, 2024 16:14

Merge remote-tracking branch 'origin/main' into HEAD

14eb36d

kovdan01 reviewed Jul 22, 2024

View reviewed changes

DanielKristofKiss added 2 commits August 8, 2024 18:45

Merge remote-tracking branch 'origin/main' into HEAD

4cb7a9c

address review comments

99b6f2b

kovdan01 reviewed Aug 23, 2024

View reviewed changes

asavonic reviewed Sep 20, 2025

View reviewed changes

DanielKristofKiss added 3 commits October 1, 2025 14:40

Merge remote-tracking branch 'origin/main' into HEAD

1e9f118

fix typo in the test

476dec8

Let's iterate less on the module flags

a1fc006

asavonic approved these changes Oct 14, 2025

View reviewed changes

MaskRay reviewed Oct 14, 2025

View reviewed changes

llvm/lib/IR/AutoUpgrade.cpp Outdated Show resolved Hide resolved

rename functions to camelBack scheme

f49192a

MaskRay approved these changes Oct 22, 2025

View reviewed changes

DanielKristofKiss merged commit 048070b into llvm:main Oct 22, 2025
10 checks passed

asavonic mentioned this pull request Oct 22, 2025

[IR] Fix Module::setModuleFlag for uniqued metadata #164580

Merged

googlewalt mentioned this pull request Oct 22, 2025

[ARM][AArch64] Move TestInputs to Inputs #164633

Merged

googlewalt added a commit that referenced this pull request Oct 22, 2025

[ARM][AArch64] Move TestInputs to Inputs (#164633)

2936852

It's standard in LLVM to have test inputs be in "Inputs". See https://llvm.org/docs/TestingGuide.html#extra-files. Fixes internal issue with #86212.


		UpgradeARCRuntime(*TheModule);

		CopyModuleAttrToFunctions(*TheModule);

[ARM][AArch64] BTI,GCS,PAC Module flag update. #86212

[ARM][AArch64] BTI,GCS,PAC Module flag update. #86212

Uh oh!

Conversation

DanielKristofKiss commented Mar 21, 2024

Uh oh!

llvmbot commented Mar 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 21, 2024

Uh oh!

llvmbot commented Mar 21, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kovdan01 left a comment

Choose a reason for hiding this comment

Uh oh!

asavonic left a comment

Choose a reason for hiding this comment

Uh oh!

asavonic Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

DanielKristofKiss Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

asavonic Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

DanielKristofKiss Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

asavonic Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

DanielKristofKiss Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

asavonic commented Oct 10, 2025

Uh oh!

asavonic commented Oct 14, 2025

Uh oh!

asavonic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DanielKristofKiss commented Oct 14, 2025

Uh oh!

DanielKristofKiss commented Oct 21, 2025

Uh oh!

Uh oh!

asavonic commented Oct 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

llvmbot commented Mar 21, 2024 •

edited

Loading