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

[Mehdi Amini via llvm-commits]

> On Jul 21, 2016, at 2:16 PM, David Majnemer <david.majnemer at gmail.com> wrote:
> 
> 
> 
> On Thu, Jul 21, 2016 at 1:55 PM, Mehdi AMINI via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> mehdi_amini added a comment.
> 
> In https://reviews.llvm.org/D22639#491753 <https://reviews.llvm.org/D22639#491753>, @bkramer wrote:
> 
> > The problem is not a Halide test but a scalability issue in MemorySSA that's making Halide unusable, consuming gigabytes of memory and taking forever. Personally, I don't think the GVNHoist pass is ready for prime time yet and needs more testing, like we did for other passes that were first rigorously tested under a flag before being enabled by default. Going directly from zero to default with a major new optimization infrastructure (MemSSA) is a recipe for disaster.
> 
> 
> I agree with all of that and I missed the fact that it was not the regular plain old GVN that we were talking here but a new (and apparently still "experimental") pass.
> With this in mind, a cl::opt flag seem appropriate and I waive my previous concern.
> Thanks for the extra information!
> 
> Having fixed a number of bugs in GVNHoist already, I think that the pass should be disabled for the 3.9 release.
> 
> I think that it would be prudent for new passes, like GVNHoist, to get disabled if there are open miscompiles against them for the first few months.
> 
> This way we let the new pass get exposed to code while also preserving the correctness of our output when we know the pass is broken.
> 
> Otherwise I fear we will never be confident in turning the pass on by default.
> 
> With all that said, I think that gvn-hoist should be disabled in trunk until pr28606 and Alina's example are resolved.

+1 for disabling by default in 3.9 (with a cl::opt to enable it), and switching the default to enabled in master as soon as (enough of) known bugs are solved.


— 
Mehdi

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160721/5cdd8fe8/attachment.html>