[bugpoint][PR29027] Reduce function attributes

[modocache]

Summary

In addition to reducing the functions in an LLVM module, bugpoint now
reduces the function attributes associated with each of the remaining
functions.

To test this, add a -bugpoint-crashfuncattr test pass, which crashes if
a function in the module has a "bugpoint-crash" attribute. A test case
demonstrates that the IR is reduced to just that one attribute.

[reames]

Looks like a good step in the right direction after some cleanup. Any chance you're interested in doing the same for argument or return attributes?

test/BugPoint/func-attrs.ll

6 ↗

(On Diff #176395)

Can you add an example which uses key=value for the crashing attribute to show that works? "bugpoint-crash"="blah" should work fine.

11 ↗

(On Diff #176395)

Can you turn this into a positive test for the one left?

tools/bugpoint/CrashDebugger.cpp

333 ↗

(On Diff #176395)

It probably makes more sense to call this Suffix?

1142 ↗

(On Diff #176395)

This should probably be outside the loop?

[modocache]

Thanks for the review, @reames! I implemented each of your suggestions, but I had a small excuse for not using the variable name Suffix.

[modocache]

tools/bugpoint/CrashDebugger.cpp

333 ↗

(On Diff #176395)

I agree, but every other implementation of doTest names this Kept... maybe we can rename them all in a future NFC commit?

[modocache]

Friendly ping! I believe I addressed all of the code review comments. @MatzeB, you created the bug report https://bugs.llvm.org/show_bug.cgi?id=29027, is this implementation what you had in mind?

[reames]

LGTM, thanks for the ping!

[modocache]

Thanks for the reviews, @reames! As a follow-up to this, I also have https://reviews.llvm.org/D55601 out for review, if you're interested -- it improves the reduction logic added in this patch.

llvm/trunk/test/BugPoint/func-attrs-keyval.ll

+; RUN: bugpoint -load %llvmshlibdir/BugpointPasses%shlibext %s -output-prefix %t -bugpoint-crashfuncattr -silence-passes
+; RUN: llvm-dis %t-reduced-simplified.bc -o - | FileCheck %s
+; REQUIRES: loadable_module
+
+; CHECK: f() #[[ATTRS:[0-9]+]]
+define void @f() #0 {
+  ret void
+}
+
+; CHECK: attributes #[[ATTRS]] = { "bugpoint-crash"="sure" }
+attributes #0 = { "bugpoint-crash"="sure" noreturn "no-frame-pointer-elim-non-leaf" }

llvm/trunk/test/BugPoint/func-attrs.ll

+; RUN: bugpoint -load %llvmshlibdir/BugpointPasses%shlibext %s -output-prefix %t -bugpoint-crashfuncattr -silence-passes
+; RUN: llvm-dis %t-reduced-simplified.bc -o - | FileCheck %s
+; REQUIRES: loadable_module
+
+; CHECK: f() #[[ATTRS:[0-9]+]]
+define void @f() #0 {
+  ret void
+}
+
+; CHECK: attributes #[[ATTRS]] = { "bugpoint-crash" }
+attributes #0 = { noinline "bugpoint-crash" "no-frame-pointer-elim-non-leaf" }

llvm/trunk/tools/bugpoint-passes/TestPasses.cpp

private:
   }
 };
 }
 
 char CrashOnTooManyCUs::ID = 0;
 static RegisterPass<CrashOnTooManyCUs>
     A("bugpoint-crash-too-many-cus",
       "BugPoint Test Pass - Intentionally crash on too many CUs");
+
+namespace {
+class CrashOnFunctionAttribute : public FunctionPass {
+public:
+  static char ID; // Pass ID, replacement for typeid
+  CrashOnFunctionAttribute() : FunctionPass(ID) {}
+
+private:
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.setPreservesAll();
+  }
+
+  bool runOnFunction(Function &F) override {
+    AttributeSet A = F.getAttributes().getFnAttributes();
+    if (A.hasAttribute("bugpoint-crash"))
+      abort();
+    return false;
+  }
+};
+} // namespace
+
+char CrashOnFunctionAttribute::ID = 0;
+static RegisterPass<CrashOnFunctionAttribute>
+    B("bugpoint-crashfuncattr", "BugPoint Test Pass - Intentionally crash on "
+                                "function attribute 'bugpoint-crash'");

llvm/trunk/tools/bugpoint/CrashDebugger.cpp

if (TestFn(BD, M.get())) {
     // module.
     Funcs.assign(Functions.begin(), Functions.end());
     return true;
   }
   return false;
 }
 
 namespace {
+/// ReduceCrashingFunctionAttributes reducer - This works by removing
+/// attributes on a particular function and seeing if the program still crashes.
+/// If it does, then keep the newer, smaller program.
+///
+class ReduceCrashingFunctionAttributes : public ListReducer<Attribute> {
+  BugDriver &BD;
+  std::string FnName;
+  BugTester TestFn;
+
+public:
+  ReduceCrashingFunctionAttributes(BugDriver &bd, const std::string &FnName,
+                                   BugTester testFn)
+      : BD(bd), FnName(FnName), TestFn(testFn) {}
+
+  Expected<TestResult> doTest(std::vector<Attribute> &Prefix,
+                              std::vector<Attribute> &Kept) override {
+    if (!Kept.empty() && TestFuncAttrs(Kept))
+      return KeepSuffix;
+    if (!Prefix.empty() && TestFuncAttrs(Prefix))
+      return KeepPrefix;
+    return NoFailure;
+  }
+
+  bool TestFuncAttrs(std::vector<Attribute> &Attrs);
+};
+}
+
+bool ReduceCrashingFunctionAttributes::TestFuncAttrs(
+    std::vector<Attribute> &Attrs) {
+  // Clone the program to try hacking it apart...
+  std::unique_ptr<Module> M = CloneModule(BD.getProgram());
+  Function *F = M->getFunction(FnName);
+
+  // Build up an AttributeList from the attributes we've been given by the
+  // reducer.
+  AttrBuilder AB;
+  for (auto A : Attrs)
+    AB.addAttribute(A);
+  AttributeList NewAttrs;
+  NewAttrs =
+      NewAttrs.addAttributes(BD.getContext(), AttributeList::FunctionIndex, AB);
+
+  // Set this new list of attributes on the function.
+  F->setAttributes(NewAttrs);
+
+  // Try running on the hacked up program...
+  if (TestFn(BD, M.get())) {
+    BD.setNewProgram(std::move(M)); // It crashed, keep the trimmed version...
+
+    // Pass along the set of attributes that caused the crash.
+    Attrs.clear();
+    for (Attribute A : NewAttrs.getFnAttributes()) {
+      Attrs.push_back(A);
+    }
+    return true;
+  }
+  return false;
+}
+
+namespace {
 /// Simplify the CFG without completely destroying it.
 /// This is not well defined, but basically comes down to "try to eliminate
 /// unreachable blocks and constant fold terminators without deciding that
 /// certain undefined behavior cuts off the program at the legs".
 void simpleSimplifyCfg(Function &F, SmallVectorImpl<BasicBlock *> &BBs) {
   if (F.empty())
     return;
 
Expected<bool> Result =
         ReduceCrashingFunctions(BD, TestFn).reduceList(Functions);
     if (Error E = Result.takeError())
       return E;
 
     if (Functions.size() < OldSize)
       BD.EmitProgressBitcode(BD.getProgram(), "reduced-function");
   }
 
+  // For each remaining function, try to reduce that function's attributes.
+  std::vector<std::string> FunctionNames;
+  for (Function &F : BD.getProgram())
+    FunctionNames.push_back(F.getName());
+
+  if (!FunctionNames.empty() && !BugpointIsInterrupted) {
+    outs() << "\n*** Attempting to reduce the number of function attributes in "
+              "the testcase\n";
+
+    unsigned OldSize = 0;
+    unsigned NewSize = 0;
+    for (std::string &Name : FunctionNames) {
+      Function *Fn = BD.getProgram().getFunction(Name);
+      assert(Fn && "Could not find funcion?");
+
+      std::vector<Attribute> Attrs;
+      for (Attribute A : Fn->getAttributes().getFnAttributes())
+        Attrs.push_back(A);
+
+      OldSize += Attrs.size();
+      Expected<bool> Result =
+          ReduceCrashingFunctionAttributes(BD, Name, TestFn).reduceList(Attrs);
+      if (Error E = Result.takeError())
+        return E;
+
+      NewSize += Attrs.size();
+    }
+
+    if (OldSize < NewSize)
+      BD.EmitProgressBitcode(BD.getProgram(), "reduced-function-attributes");
+  }
+
   // Attempt to change conditional branches into unconditional branches to
   // eliminate blocks.
   if (!DisableSimplifyCFG && !BugpointIsInterrupted) {
     std::vector<const BasicBlock *> Blocks;
     for (Function &F : BD.getProgram())
       for (BasicBlock &BB : F)
         Blocks.push_back(&BB);
     unsigned OldSize = Blocks.size();