[llvm] r300200 - Re-apply "[GVNHoist] Move GVNHoist to function simplification part of pipeline."

Aditya K via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 24 11:02:05 PDT 2017


>If any one decides to revert this, I'd like to request that you attempt
>to do so by just turning off GVNHoist by default instead of reverting
>r300200 if possible.  I believe the current location in the pass
>pipeline is the desired one, and reverting r300200 will just move it
>back to being done before inlining, which is likely just hiding bugs.
>
>FWIW, before re-enabling this most recently I did correctness testing on
>X86 and AArch64 of the test-suite, SPEC and several other benchmarks, as
>well as bootstrapping clang, all without issue.

I have also tested the recent GVNHoist and it bootstraps, passes SPEC2006. Maybe it is related to the bug https://bugs.llvm.org/show_bug.cgi?id=30806
If there is a test case to reproduce the bug I'll be glad to fix it.


Thanks,
-Aditya

>
>On 4/23/2017 12:45 PM, Philip Reames wrote:
>> If this does turn out to be the problematic commit, I'll ask that we
>> do not re-enable GVNHoist without some llvm-dev discussion of what
>> we've done to test it. I've literally lost track of the number of
>> times this has been enabled and disabled by now.  There's clearly a
>> problem here; it's time we discuss it and what needs done to address
>> it going forward.
>>
>> Philip
>>
>> On 04/21/2017 08:24 PM, Chandler Carruth via llvm-commits wrote:
>>> FYI and just as a heads up, we're seeing buggy hoisting leading to
>>> null pointers being accessed, and this commit is in the range and
>>> seems very likely to be responsible. We're working on getting this
>>> isolated enough to be sure, but wanted to mention it in case anyone
>>> else is similarly looking at issues after this commit.

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


More information about the llvm-commits mailing list