[PATCH] D22639: Add flag to PassManagerBuilder to disable GVN Hoist Pass.

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

In https://reviews.llvm.org/D22639#491781, @sebpop wrote:

> We already have a flag to disable the GVN-hoist pass:
>
>   static cl::opt<int>
>      MaxHoistedThreshold("gvn-max-hoisted", cl::Hidden, cl::init(-1),
>                          cl::desc("Max number of instructions to hoist "
>                                   "(default unlimited = -1)"));
>   
>
> Please add "-mllvm -gvn-max-hoisted=0" to the failing tests.


IMO, this is not the appropriate way to disable things not ready for prime time.

We should typically be disabling passes with a commandline flag *and* an option in the PassManagerBuilder because without that, library users of LLVM like Halide and others are left out in the cold.

That said, I certainly agree with Mehdi that we should design a better interface to the flags used by the PassManagerBuilder, but I don't think we should do that right now -- the goal here is to stop hurting users of LLVM that are completely broken by a new pass.

(And IMO, it should be disabled by default, and it should get enabled after reports from a diverse group of users report success. It certainly shouldn't be enabled by default weeks before a release branch.)


https://reviews.llvm.org/D22639