[bugpoint] Reduce metadata that does not contribute to crash.

[fhahn]

Summary

Add a new reducer that drops metadata that does not contribute to the
crash from instructions.

It adjusts the metadata.ll test case, as now also the instruction level
metadata will get dropped.

[fhahn]

Fix typo

[reames]

llvm/tools/bugpoint-passes/TestPasses.cpp

178

*range* metadata

llvm/tools/bugpoint/CrashDebugger.cpp

852

Er, it really looks like you should be iterating the instruction set above?

[fhahn]

Update patch to retain metadata for selected instructions. Add additional test to make sure we correctly drop metadata from some instructions, while retaining it for others.

[fhahn]

llvm/tools/bugpoint-passes/TestPasses.cpp

178

I've slightly changed the test pass, to additionally require the operand to be an fadd instruction, to avoid the call being the only remaining instruction.

llvm/tools/bugpoint/CrashDebugger.cpp

852

Yes, we should retain metadata on all instructions in Insts/Instructions! This seems to be consistent with other ListReducers.

[reames]

LGTM

llvm/test/BugPoint/metadata.ll

 ; REQUIRES: plugins
 ; RUN: bugpoint -load %llvmshlibdir/BugpointPasses%shlibext %s -output-prefix %t -bugpoint-crashcalls -silence-passes -disable-namedmd-remove -disable-strip-debuginfo -disable-strip-debug-types > /dev/null
 ; RUN: llvm-dis %t-reduced-simplified.bc -o - | FileCheck %s
 ;
 ; RUN: bugpoint -load %llvmshlibdir/BugpointPasses%shlibext %s -output-prefix %t-nodebug -bugpoint-crashcalls -silence-passes -disable-namedmd-remove > /dev/null
 ; RUN: llvm-dis %t-nodebug-reduced-simplified.bc -o - | FileCheck %s --check-prefix=NODEBUG
 ;
 ; RUN: bugpoint -load %llvmshlibdir/BugpointPasses%shlibext %s -output-prefix %t-notype -bugpoint-crashcalls -silence-passes -disable-namedmd-remove -disable-strip-debuginfo > /dev/null
 ; RUN: llvm-dis %t-notype-reduced-simplified.bc -o - | FileCheck %s --check-prefix=NOTYPE
 ;
-; Bugpoint should keep the call's metadata attached to the call.
+; Bugpoint can drop the metadata on the call, as it does not contrinute to the crash.
 
-; CHECK: call void @foo(), !dbg ![[LOC:[0-9]+]], !attach ![[CALL:[0-9]+]]
-; NODEBUG: call void @foo(), !attach ![[CALL:[0-9]+]]
-; NOTYPE: call void @foo(), !dbg ![[LOC:[0-9]+]], !attach ![[CALL:[0-9]+]]
-; NODEBUG-NOT: call void @foo(), !attach ![[CALL:[0-9]+]]
+; CHECK: call void @foo()
+; NODEBUG: call void @foo()
+; NOTYPE: call void @foo()
+; NODEBUG-NOT: call void @foo()
 ; NOTYPE-NOT: !DIBasicType
 ; NOTYPE: !DICompileUnit
 ; NOTYPE-NOT: !DIBasicType
-; CHECK-DAG: ![[LOC]] = !DILocation(line: 104, column: 105, scope: ![[SCOPE:[0-9]+]])
-; CHECK-DAG: ![[SCOPE]] = distinct !DISubprogram(name: "test",{{.*}}file: ![[FILE:[0-9]+]]
-; CHECK-DAG: ![[FILE]] = !DIFile(filename: "source.c", directory: "/dir")
-; CHECK-DAG: ![[CALL]] = !{!"the call to foo"}
 
 %rust_task = type {}
-define void @test(i32* %a, i8* %b) {
+define void @test(i32* %a, i8* %b) !dbg !9 {
     %s = mul i8 22, 9, !attach !0, !dbg !10
     store i8 %s, i8* %b, !attach !1, !dbg !11
     call void @foo(), !attach !2, !dbg !12
     store i32 7, i32* %a, !attach !3, !dbg !13
     %t = add i32 0, 5, !attach !4, !dbg !14
     ret void
 }
 

llvm/test/BugPoint/retain-crashing-metadata.ll

+; REQUIRES: plugins
+; RUN: bugpoint -load %llvmshlibdir/BugpointPasses%shlibext %s -output-prefix %t-notype -bugpoint-crashmetadata -silence-passes > /dev/null
+; RUN: llvm-dis %t-notype-reduced-simplified.bc -o - | FileCheck %s
+;
+; Make sure BugPoint retains metadata contributing to a crash.
+
+; CHECK-LABEL: define void @test2(float %f) {
+; CHECK-NEXT: %arg = fadd float %f, 1.000000e+01
+; CHECK-NOT: !fpmath
+; CHECK-NEXT: %x = call float @llvm.fabs.f32(float %arg), !fpmath [[FPMATH:![0-9]+]]
+; CHECK-NEXT: ret void
+
+; CHECK: [[FPMATH]] = !{float 2.500000e+00}
+define void @test2(float %f) {
+    %arg = fadd float %f, 1.000000e+01, !fpmath !0
+    %x = call float @llvm.fabs.f32(float %arg), !fpmath !0
+    ret void
+}
+
+declare float @llvm.fabs.f32(float)
+
+!0 = !{float 2.500000e+00}

llvm/tools/bugpoint-passes/TestPasses.cpp

 //===- TestPasses.cpp - "buggy" passes used to test bugpoint --------------===//
 //
+//
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file contains "buggy" passes that are used to test bugpoint, to check
 // that it is narrowing down testcases correctly.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Constant.h"
+#include "llvm/IR/InstIterator.h"
 #include "llvm/IR/InstVisitor.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Type.h"
 #include "llvm/Pass.h"
 
+#include "llvm/IR/PatternMatch.h"
+using namespace llvm::PatternMatch;
 using namespace llvm;
 
 namespace {
   /// CrashOnCalls - This pass is used to test bugpoint.  It intentionally
   /// crashes on any call instructions.
   class CrashOnCalls : public BasicBlockPass {
   public:
     static char ID; // Pass ID, replacement for typeid
private:
   }
 };
 } // namespace
 
 char CrashOnFunctionAttribute::ID = 0;
 static RegisterPass<CrashOnFunctionAttribute>
     B("bugpoint-crashfuncattr", "BugPoint Test Pass - Intentionally crash on "
                                 "function attribute 'bugpoint-crash'");
+
+namespace {
+class CrashOnMetadata : public FunctionPass {
+public:
+  static char ID; // Pass ID, replacement for typeid
+  CrashOnMetadata() : FunctionPass(ID) {}
+
+private:
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.setPreservesAll();
+  }
+
+  // Crash on fabs calls with fpmath metdata and an fadd as argument. This
+  // ensures the fadd instruction sticks around and we can check that the
+  // metadata there is dropped correctly.
+  bool runOnFunction(Function &F) override {
+    for (Instruction &I : instructions(F))
+      if (match(&I, m_FAbs(m_FAdd(m_Value(), m_Value()))) &&
+          I.hasMetadata("fpmath"))
+        abort();
+    return false;
+  }
+};
+} // namespace
+
+char CrashOnMetadata::ID = 0;
+static RegisterPass<CrashOnMetadata>
+    C("bugpoint-crashmetadata",
+      "BugPoint Test Pass - Intentionally crash on "
+      "fabs calls with fpmath metadata and an fadd as argument");

llvm/tools/bugpoint/CrashDebugger.cpp

 //===----------------------------------------------------------------------===//
 
 #include "BugDriver.h"
 #include "ListReducer.h"
 #include "ToolRunner.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
-#include "llvm/Transforms/Utils/Local.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/InstIterator.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/ValueSymbolTable.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Transforms/Scalar.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Cloning.h"
+#include "llvm/Transforms/Utils/Local.h"
 #include <set>
 using namespace llvm;
 
 namespace {
 cl::opt<bool> KeepMain("keep-main",
                        cl::desc("Force function reduction to keep main"),
                        cl::init(false));
 cl::opt<bool> NoGlobalRM("disable-global-remove",
for (Instruction *Inst : Instructions)
       Insts.push_back(Inst);
     return true;
   }
   // It didn't crash, try something else.
   return false;
 }
 
 namespace {
+/// ReduceCrashingMetadata reducer - This works by removing all metadata from
+/// the specified instructions.
+///
+class ReduceCrashingMetadata : public ListReducer<Instruction *> {
+  BugDriver &BD;
+  BugTester TestFn;
+
+public:
+  ReduceCrashingMetadata(BugDriver &bd, BugTester testFn)
+      : BD(bd), TestFn(testFn) {}
+
+  Expected<TestResult> doTest(std::vector<Instruction *> &Prefix,
+                              std::vector<Instruction *> &Kept) override {
+    if (!Kept.empty() && TestInsts(Kept))
+      return KeepSuffix;
+    if (!Prefix.empty() && TestInsts(Prefix))
+      return KeepPrefix;
+    return NoFailure;
+  }
+
+  bool TestInsts(std::vector<Instruction *> &Prefix);
+};
+} // namespace
+
+bool ReduceCrashingMetadata::TestInsts(std::vector<Instruction *> &Insts) {
+  // Clone the program to try hacking it apart...
+  ValueToValueMapTy VMap;
+  std::unique_ptr<Module> M = CloneModule(BD.getProgram(), VMap);
+
+  // Convert list to set for fast lookup...
+  SmallPtrSet<Instruction *, 32> Instructions;
+  for (Instruction *I : Insts)
+    Instructions.insert(cast<Instruction>(VMap[I]));
+
+  outs() << "Checking for crash with metadata retained from "
+         << Instructions.size();
+  if (Instructions.size() == 1)
+    outs() << " instruction: ";
+  else
+    outs() << " instructions: ";
+
+  // Try to drop instruction metadata from all instructions, except the ones
+  // selected in Instructions.
+  for (Function &F : *M)
+    for (Instruction &Inst : instructions(F)) {
+      if (Instructions.find(&Inst) == Instructions.end()) {
+        Inst.dropUnknownNonDebugMetadata();
+        Inst.setDebugLoc({});
+      }
+    }
+
+  // Verify that this is still valid.
+  legacy::PassManager Passes;
+  Passes.add(createVerifierPass(/*FatalErrors=*/false));
+  Passes.run(*M);
+
+  // Try running on the hacked up program...
+  if (TestFn(BD, M.get())) {
+    BD.setNewProgram(std::move(M)); // It crashed, keep the trimmed version...
+
+    // Make sure to use instruction pointers that point into the now-current
+    // module, and that they don't include any deleted blocks.
+    Insts.clear();
+    for (Instruction *I : Instructions)
+      Insts.push_back(I);
+    return true;
+  }
+  // It didn't crash, try something else.
+  return false;
+}
+
+namespace {
 // Reduce the list of Named Metadata nodes. We keep this as a list of
 // names to avoid having to convert back and forth every time.
 class ReduceCrashingNamedMD : public ListReducer<std::string> {
   BugDriver &BD;
   BugTester TestFn;
 
 public:
   ReduceCrashingNamedMD(BugDriver &bd, BugTester testFn)
for (Module::const_iterator FI = BD.getProgram().begin(),
           }
 
     if (InstructionsToSkipBeforeDeleting) {
       InstructionsToSkipBeforeDeleting = 0;
       goto TryAgain;
     }
 
   } while (Simplification);
+
+  // Attempt to drop metadata from instructions that does not contribute to the
+  // crash.
+  if (!BugpointIsInterrupted) {
+    std::vector<Instruction *> Insts;
+    for (Function &F : BD.getProgram())
+      for (Instruction &I : instructions(F))
+        Insts.push_back(&I);
+
+    Expected<bool> Result =
+        ReduceCrashingMetadata(BD, TestFn).reduceList(Insts);
+    if (Error E = Result.takeError())
+      return E;
+  }
+
   BD.EmitProgressBitcode(BD.getProgram(), "reduced-instructions");
   return Error::success();
 }
 
 /// DebugACrash - Given a predicate that determines whether a component crashes
 /// on a program, try to destructively reduce the program while still keeping
 /// the predicate true.
 static Error DebugACrash(BugDriver &BD, BugTester TestFn) {