[llvm] b3fd3a9 - [IR] Allow absence for Min module flags and make AArch64 BTI/PAC-RET flags backward compatible

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 18 09:35:16 PDT 2022


Author: Fangrui Song
Date: 2022-07-18T09:35:12-07:00
New Revision: b3fd3a9ac3eb31bc478abeefe64c05bfbecd3233

URL: https://github.com/llvm/llvm-project/commit/b3fd3a9ac3eb31bc478abeefe64c05bfbecd3233
DIFF: https://github.com/llvm/llvm-project/commit/b3fd3a9ac3eb31bc478abeefe64c05bfbecd3233.diff

LOG: [IR] Allow absence for Min module flags and make AArch64 BTI/PAC-RET flags backward compatible

D123493 introduced llvm::Module::Min to encode module flags metadata for AArch64
BTI/PAC-RET. llvm::Module::Min does not take effect when the flag is absent in
one module. This behavior is misleading and does not address backward
compatibility problems (when a bitcode with "branch-target-enforcement"==1 and
another without the flag are merged, the merge result is 1 instead of 0).

To address the problems, require Min flags to be non-negative and treat absence
as having a value of zero. For an old bitcode without
"branch-target-enforcement"/"sign-return-address", its value is as if 0.

Differential Revision: https://reviews.llvm.org/D129911

Added: 
    llvm/test/Linker/module-flags-min.ll

Modified: 
    llvm/docs/LangRef.rst
    llvm/lib/IR/Verifier.cpp
    llvm/lib/Linker/IRMover.cpp
    llvm/test/Verifier/module-flags-1.ll

Removed: 
    


################################################################################
diff  --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index f641a3cfd4346..9464b20619ab6 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -7320,6 +7320,11 @@ The following behaviors are supported:
      - **Max**
            Takes the max of the two values, which are required to be integers.
 
+   * - 8
+     - **Min**
+           Takes the min of the two values, which are required to be non-negative integers.
+           An absent module flag is treated as having the value 0.
+
 It is an error for a particular unique flag ID to have multiple behaviors,
 except in the case of **Require** (which adds restrictions on another metadata
 value) or **Override**.

diff  --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 806bce7c8fc0a..edbe76cc9cac8 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -1624,8 +1624,10 @@ Verifier::visitModuleFlag(const MDNode *Op,
     break;
 
   case Module::Min: {
-    Check(mdconst::dyn_extract_or_null<ConstantInt>(Op->getOperand(2)),
-          "invalid value for 'min' module flag (expected constant integer)",
+    auto *V = mdconst::dyn_extract_or_null<ConstantInt>(Op->getOperand(2));
+    Check(V && V->getValue().isNonNegative(),
+          "invalid value for 'min' module flag (expected constant non-negative "
+          "integer)",
           Op->getOperand(2));
     break;
   }

diff  --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index 7ccaba7e1c5c4..e31faf6422edd 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -1273,14 +1273,19 @@ Error IRLinker::linkModuleFlagsMetadata() {
   // First build a map of the existing module flags and requirements.
   DenseMap<MDString *, std::pair<MDNode *, unsigned>> Flags;
   SmallSetVector<MDNode *, 16> Requirements;
+  SmallVector<unsigned, 0> Mins;
+  DenseSet<MDString *> SeenMin;
   for (unsigned I = 0, E = DstModFlags->getNumOperands(); I != E; ++I) {
     MDNode *Op = DstModFlags->getOperand(I);
-    ConstantInt *Behavior = mdconst::extract<ConstantInt>(Op->getOperand(0));
+    uint64_t Behavior =
+        mdconst::extract<ConstantInt>(Op->getOperand(0))->getZExtValue();
     MDString *ID = cast<MDString>(Op->getOperand(1));
 
-    if (Behavior->getZExtValue() == Module::Require) {
+    if (Behavior == Module::Require) {
       Requirements.insert(cast<MDNode>(Op->getOperand(2)));
     } else {
+      if (Behavior == Module::Min)
+        Mins.push_back(I);
       Flags[ID] = std::make_pair(Op, I);
     }
   }
@@ -1296,6 +1301,7 @@ Error IRLinker::linkModuleFlagsMetadata() {
     unsigned DstIndex;
     std::tie(DstOp, DstIndex) = Flags.lookup(ID);
     unsigned SrcBehaviorValue = SrcBehavior->getZExtValue();
+    SeenMin.insert(ID);
 
     // If this is a requirement, add it and continue.
     if (SrcBehaviorValue == Module::Require) {
@@ -1309,6 +1315,10 @@ Error IRLinker::linkModuleFlagsMetadata() {
 
     // If there is no existing flag with this ID, just add it.
     if (!DstOp) {
+      if (SrcBehaviorValue == Module::Min) {
+        Mins.push_back(DstModFlags->getNumOperands());
+        SeenMin.erase(ID);
+      }
       Flags[ID] = std::make_pair(SrcOp, DstModFlags->getNumOperands());
       DstModFlags->addOperand(SrcOp);
       continue;
@@ -1467,6 +1477,20 @@ Error IRLinker::linkModuleFlagsMetadata() {
 
   }
 
+  // For the Min behavior, set the value to 0 if either module does not have the
+  // flag.
+  for (auto Idx : Mins) {
+    MDNode *Op = DstModFlags->getOperand(Idx);
+    MDString *ID = cast<MDString>(Op->getOperand(1));
+    if (!SeenMin.count(ID)) {
+      ConstantInt *V = mdconst::extract<ConstantInt>(Op->getOperand(2));
+      Metadata *FlagOps[] = {
+          Op->getOperand(0), ID,
+          ConstantAsMetadata::get(ConstantInt::get(V->getType(), 0))};
+      DstModFlags->setOperand(Idx, MDNode::get(DstM.getContext(), FlagOps));
+    }
+  }
+
   // Check all of the requirements.
   for (unsigned I = 0, E = Requirements.size(); I != E; ++I) {
     MDNode *Requirement = Requirements[I];

diff  --git a/llvm/test/Linker/module-flags-min.ll b/llvm/test/Linker/module-flags-min.ll
new file mode 100644
index 0000000000000..71512977b2616
--- /dev/null
+++ b/llvm/test/Linker/module-flags-min.ll
@@ -0,0 +1,27 @@
+; RUN: rm -rf %t && split-file %s %t && cd %t
+; RUN: llvm-link a.ll b.ll -S -o - 2>&1 | FileCheck %s
+
+; CHECK:      !0 = !{i32 8, !"foo", i16 2}
+; CHECK-NEXT: !1 = !{i32 8, !"bar", i64 3}
+; CHECK-NEXT: !2 = !{i32 8, !"only_in_a", i32 0}
+; CHECK-NEXT: !3 = !{i32 8, !"required_in_b", i32 3}
+; CHECK-NEXT: !4 = !{i32 8, !"only_in_b", i32 0}
+; CHECK-NEXT: !5 = !{i32 3, !"require", !6}
+; CHECK-NEXT: !6 = !{!"required_in_b", i32 3}
+
+;--- a.ll
+!0 = !{ i32 8, !"foo", i16 2 }
+!1 = !{ i32 8, !"bar", i64 4 }
+!2 = !{ i32 8, !"only_in_a", i32 4 }
+!3 = !{ i32 8, !"required_in_b", i32 3 }
+
+!llvm.module.flags = !{ !0, !1, !2, !3 }
+
+;--- b.ll
+!0 = !{ i32 8, !"foo", i16 3 }
+!1 = !{ i32 8, !"bar", i64 3 }
+!2 = !{ i32 8, !"only_in_b", i32 3 }
+!3 = !{ i32 8, !"required_in_b", i32 3 }
+!4 = !{ i32 3, !"require", !{!"required_in_b", i32 3} }
+
+!llvm.module.flags = !{ !0, !1, !2, !3, !4 }

diff  --git a/llvm/test/Verifier/module-flags-1.ll b/llvm/test/Verifier/module-flags-1.ll
index a3e50c59f18e4..3dfc2a31462ef 100644
--- a/llvm/test/Verifier/module-flags-1.ll
+++ b/llvm/test/Verifier/module-flags-1.ll
@@ -46,8 +46,10 @@
 !19 = !{i32 7, !"max", !"max"}
 
 ; Check that any 'min' module flags are valid.
-; CHECK: invalid value for 'min' module flag (expected constant integer)
+; CHECK: invalid value for 'min' module flag (expected constant non-negative integer)
 !20 = !{i32 8, !"min", !"min"}
+; CHECK: invalid value for 'min' module flag (expected constant non-negative integer)
+!21 = !{i32 8, !"min", i32 -1}
 
 ; Check that any 'require' module flags are valid.
 ; CHECK: invalid requirement on flag, flag is not present in module
@@ -62,4 +64,4 @@
 
 !llvm.module.flags = !{
   !0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11, !12, !13, !14, !15,
-  !16, !17, !18, !19, !20 }
+  !16, !17, !18, !19, !20, !21 }


        


More information about the llvm-commits mailing list