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

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 26 17:45:25 PDT 2017


Disabled in r301505.

On Wed, Apr 26, 2017 at 5:01 PM Chandler Carruth <chandlerc at gmail.com>
wrote:

> Finally got a good test case for this:
>
> http://llvm.org/PR32821
>
> I'm going to flip the flag to "off" as suggested by Geoff, I completely
> agree with that rather than moving it around.
>
> On Mon, Apr 24, 2017 at 11:02 AM Aditya K <hiraditya at msn.com> wrote:
>
>>
>> >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/20170427/90701e14/attachment.html>


More information about the llvm-commits mailing list