[llvm] 7cd0fd6 - llvm-reduce: Avoid invalid attribute reduction on optnone functions

[Matt Arsenault via llvm-commits]

Author: Matt Arsenault
Date: 2023-01-03T15:48:14-05:00
New Revision: 7cd0fd6f519558fcccf07d7cbf14237f2c5182fe

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

LOG: llvm-reduce: Avoid invalid attribute reduction on optnone functions

We have this ridiculous restriction that optnone requires noinline,
so the pair needs to be removed if we want to remove noinline.

Added: 
    llvm/test/tools/llvm-reduce/reduce-attributes-optnone-noinline.ll

Modified: 
    llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/tools/llvm-reduce/reduce-attributes-optnone-noinline.ll b/llvm/test/tools/llvm-reduce/reduce-attributes-optnone-noinline.ll
new file mode 100644
index 000000000000..6229a3f9c2d8
--- /dev/null
+++ b/llvm/test/tools/llvm-reduce/reduce-attributes-optnone-noinline.ll
@@ -0,0 +1,38 @@
+; RUN: llvm-reduce --abort-on-invalid-reduction --delta-passes=attributes --test FileCheck --test-arg -check-prefixes=INTERESTING,INTERESTING-NOINLINE --test-arg %s --test-arg --input-file %s -o %t.0
+; RUN: FileCheck --check-prefix=RESULT-NOINLINE %s < %t.0
+
+; RUN: llvm-reduce --abort-on-invalid-reduction --delta-passes=attributes --test FileCheck --test-arg -check-prefixes=INTERESTING,INTERESTING-OPTNONE --test-arg %s --test-arg --input-file %s -o %t.1
+; RUN: FileCheck --check-prefix=RESULT-OPTNONE %s < %t.1
+
+
+; Make sure this doesn't hit the "Attribute 'optnone' requires
+; 'noinline'!" verifier error. optnone can be dropped separately from
+; noinline, but removing noinline requires removing the pair together.
+
+
+; INTERESTING: @keep_func() [[KEEP_ATTRS:#[0-9]+]]
+; RESULT-NOINLINE: define void @keep_func() [[KEEP_ATTRS:#[0-9]+]] {
+; RESULT-OPTNONE: define void @keep_func() [[KEEP_ATTRS:#[0-9]+]] {
+define void @keep_func() #0 {
+  ret void
+}
+
+; Both should be removed together
+; INTERESTING: @drop_func()
+; RESULT-NOINLINE: define void @drop_func() {
+; RESULT-OPTNONE: define void @drop_func() {
+define void @drop_func() #0 {
+  ret void
+}
+
+; RESULT-NOINLINE: attributes [[KEEP_ATTRS]] = { noinline }
+; RESULT-OPTNONE: attributes [[KEEP_ATTRS]] = { noinline optnone }
+
+
+; INTERESTING-NOINLINE: attributes [[KEEP_ATTRS]] =
+; INTERESTING-NOINLINE-SAME: noinline
+
+; INTERESTING-OPTNONE: attributes [[KEEP_ATTRS]] =
+; INTERESTING-OPTNONE-SAME: optnone
+
+attributes #0 = { noinline optnone }

diff  --git a/llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp b/llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp
index 2ff274f5f155..56503676920b 100644
--- a/llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp
+++ b/llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp
@@ -42,7 +42,7 @@ using namespace llvm;
 
 namespace {
 
-using AttrPtrVecTy = std::vector<const Attribute *>;
+using AttrPtrVecTy = std::vector<Attribute>;
 using AttrPtrIdxVecVecTy = std::pair<unsigned, AttrPtrVecTy>;
 using AttrPtrVecVecTy = SmallVector<AttrPtrIdxVecVecTy, 3>;
 
@@ -94,24 +94,46 @@ class AttributeRemapper : public InstVisitor<AttributeRemapper> {
     }
   }
 
+  // FIXME: Should just directly use AttrBuilder instead of going through
+  // AttrPtrVecTy
   void visitAttributeSet(const AttributeSet &AS,
                          AttrPtrVecTy &AttrsToPreserve) {
     assert(AttrsToPreserve.empty() && "Should not be sharing vectors.");
     AttrsToPreserve.reserve(AS.getNumAttributes());
-    for (const Attribute &A : AS)
+
+    // Optnone requires noinline, so removing noinline requires removing the
+    // pair.
+    Attribute NoInline = AS.getAttribute(Attribute::NoInline);
+    bool RemoveNoInline = false;
+    if (NoInline.isValid()) {
+      RemoveNoInline = !O.shouldKeep();
+      if (!RemoveNoInline)
+        AttrsToPreserve.emplace_back(NoInline);
+    }
+
+    for (Attribute A : AS) {
+      if (A.isEnumAttribute()) {
+        Attribute::AttrKind Kind = A.getKindAsEnum();
+        if (Kind == Attribute::NoInline)
+          continue;
+
+        if (RemoveNoInline && Kind == Attribute::OptimizeNone)
+          continue;
+      }
+
       if (O.shouldKeep())
-        AttrsToPreserve.emplace_back(&A);
+        AttrsToPreserve.emplace_back(A);
+    }
   }
 };
 
 } // namespace
 
-AttributeSet
-convertAttributeRefToAttributeSet(LLVMContext &C,
-                                  ArrayRef<const Attribute *> Attributes) {
+AttributeSet convertAttributeRefToAttributeSet(LLVMContext &C,
+                                               ArrayRef<Attribute> Attributes) {
   AttrBuilder B(C);
-  for (const Attribute *A : Attributes)
-    B.addAttribute(*A);
+  for (Attribute A : Attributes)
+    B.addAttribute(A);
   return AttributeSet::get(C, B);
 }