Skip to content

Conversation

@DanielKristofKiss
Copy link
Member

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 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
@llvmbot
Copy link
Member

llvmbot commented Mar 21, 2024

@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
Copy link
Member

llvmbot commented Mar 21, 2024

@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
Copy link
Member

llvmbot commented Mar 21, 2024

@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.
Copy link
Contributor

@kovdan01 kovdan01 left a comment

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

@asavonic asavonic left a comment

Choose a reason for hiding this comment

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

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.

if (!T.isThumb() && !T.isARM() && !T.isAArch64())
return;

if (isModuleAttributeTwo(M, "branch-target-enforcement"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, will be done.

// 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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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


UpgradeARCRuntime(*TheModule);

CopyModuleAttrToFunctions(*TheModule);
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

@asavonic
Copy link
Collaborator

@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
Copy link
Collaborator

@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.

Copy link
Collaborator

@asavonic asavonic left a comment

Choose a reason for hiding this comment

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

LGTM

@DanielKristofKiss
Copy link
Member Author

@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
Copy link
Member Author

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

@DanielKristofKiss DanielKristofKiss merged commit 048070b into llvm:main Oct 22, 2025
10 checks passed
@asavonic
Copy link
Collaborator

@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.

googlewalt added a commit that referenced this pull request Oct 22, 2025
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.
asavonic added a commit that referenced this pull request Oct 24, 2025
`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.
dvbuka pushed a commit to dvbuka/llvm-project that referenced this pull request Oct 27, 2025
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
dvbuka pushed a commit to dvbuka/llvm-project that referenced this pull request Oct 27, 2025
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.
dvbuka pushed a commit to dvbuka/llvm-project that referenced this pull request Oct 27, 2025
`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.
Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Oct 29, 2025
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
Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Oct 29, 2025
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.
Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Oct 29, 2025
`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.
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
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
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
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.
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
`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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants