<div dir="ltr"><div class="gmail_extra"><div><div class="gmail_signature">On Tue, Nov 22, 2016 at 10:27 AM, Jack Howarth via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span> wrote:<br></div></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">>>On Fri, Nov 18, 2016 at 8:43 AM, Philip Reames via llvm-dev<br>
><llvm-dev at <a href="http://lists.llvm.org" rel="noreferrer" target="_blank">lists.llvm.org</a>> wrote:<br>
>> Glad to see this landing!  It's been a long time coming.<br>
>><br>
>> Once this is in, please do not turn it on by default immediately.  Let's<br>
>> call for volunteers to find some of the most egregious miscompiles, fix<br>
>> them, and then turn this on by default.<br>
>><br>
>There are no immediate plans to enable NewGVN by default (at least,<br>
>not in the near future). In fact, the mail that I originally wrote<br>
>doesn't at all mention the switch, neither any follow-ups from me or<br>
>Daniel, so, I'm not entirely sure where you got that idea from. If you<br>
>take a look more closely (at the mail, or a the patch), you'll realize<br>
>that "key" pieces that are in old GVN are still missing. The most<br>
>noticeable are PRE and load coercion. In other words, the patch<br>
>proposed is not (yet) on par with what the current GVN does (although<br>
>all the missing pieces are already implemented out-of-tree).<br>
>Also, let me try to clarify one point. This is already a call for<br>
>volunteers. If you feel adventurous, you can download the<br>
>patch/apply/test/report issues. I can and I will spend time<br>
i>ntegrating the rest of the work and fix all the reported<br>
>bugs/miscompiles. If there's something that can we do in a cleaner<br>
>way, a discussion will happen on the mailing list/on the review thread<br>
>and everybody will have a chance to comment, as it's happening for the<br>
>initial patch (and as I always try to do).<br>
>Once the first patch lands, I'll commit a temporary cl::opt to enable<br>
>NewGVN for those interested in testing and send another CFT e-mail.<br>
>FWIW, The patch had already a round of light testing internally. Of<br>
>course, this is not enough or indicative of its maturity/robustness. I<br>
>plan to have it tested more carefully inside my organization in<br>
>parallel.<br>
>That said, thanks for you input.<br>
<br>
Why even commit the patch to enable the pass into trunk. Just leave it<br>
up on Phabricator for testers to apply locally to their tree?<br>
             Jack<br>
<br></blockquote><div><br></div><div>Because that increase friction to testing it. Asking people to pass -mllvm -newgvn will end up with a lot more testing than "please apply this patch and recompile everything".</div><div><br></div><div>- Michael Spencer<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
>--<br>
>Davide<br>
______________________________<wbr>_________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br>
</blockquote></div><br></div></div>