[PATCH] D154348: Ignore modified attribute list if it yields invalid IR

[Manish Kausik H via Phabricator via llvm-commits]

Nirhar added inline comments.


================
Comment at: llvm/tools/bugpoint/CrashDebugger.cpp:374
+  Passes.push_back("verify");
+  std::unique_ptr<Module> New = BD.runPassesOn(M.get(), Passes);
+  if (!New)
----------------
arsenm wrote:
> modocache wrote:
> > modocache wrote:
> > > Nirhar wrote:
> > > > lattner wrote:
> > > > > Can you use `llvm::verifyModule` instead of running the pass?
> > > > Other parts of the `CrashDebugger` run the pass to verify llvm IR, hence I kept it the same. Let me know if I should still change to `llvm::verifyModule`.
> > > You bring up a good point, @Nirhar -- there's actually a lot of duplication in this file. I count 7 spots where the verifier pass is run, and this makes 8! Here's an example: https://github.com/llvm/llvm-project/blob/b4b532a9562a1ebca347edc566363fba0531115b/llvm/tools/bugpoint/CrashDebugger.cpp#L512-L520
> > > 
> > > I agree that it would be nicer to use `llvm::verifyModule` here; it accomplishes the same thing, but in a much simpler way.
> > > 
> > > (And, I think it would be even better, in a separate patch, to change the other 7 spots to use `verifyModule` too -- and, ideally, have them print "verify failed!" and exit, like many of them seem to do.)
> > > ideally, have them print "verify failed!" and exit, like many of them seem to do
> > 
> > Sorry, typo here. I meant to write that it would be nice to 1) have these other spots use `verifyModule`, AND 2) unify them with a static helper function or something, such that we had one function that could verify a module and print "verify failed!" if verification failed. Right now that logic is duplicated a ton in this file.
> It would be easier yet to just delete bugpoint altogether and switch to using llvm-reduce. The attribute reducer there takes care to avoid introducing invalid attribute combinations 
I agree with you, let me refactor the code and then update the diff to incorporate your suggestions.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154348/new/

https://reviews.llvm.org/D154348