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

[David Majnemer via llvm-commits]

On Thu, Jul 21, 2016 at 1:55 PM, Mehdi AMINI via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> mehdi_amini added a comment.
>
> In 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.


>
>
> https://reviews.llvm.org/D22639
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160721/15709113/attachment.html>