[llvm-dev] Removal of Global SRoA

[Sjoerd Meijer via llvm-dev]

Hello,

I am interested in this if this improves SPEC, which I didn't know to be honest.

If this doesn't trigger anymore, I am willing to look into that (not immediately, but at some point), but if the opinion is that the current state and implementation is beyond repair and need to be removed then that is interesting and probably best be discussed here as there are very few reviewers on those changes, so this message may not have reached all who are interested.

Cheers,
Sjoerd.
________________________________
From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Fāng-ruì Sòng via llvm-dev <llvm-dev at lists.llvm.org>
Sent: 18 May 2021 22:06
To: Chris Lattner <clattner at nondot.org>
Cc: llvm-dev <llvm-dev at lists.llvm.org>
Subject: Re: [llvm-dev] Removal of Global SRoA

Hi Chris,

[I took my second dose of vaccine yesterday and felt under the weather, so I may be slow on replies.]

On 2021-05-18, Chris Lattner wrote:
>Hi Fangrui,
>
>In this patch you removed an optimization outright: https://reviews.llvm.org/rG129f466e222e
>
>I didn’t see this get discussed, and the only justification seems to be that “it doesn’t does not trigger at all when bootstrapping clang”.

Sorry but I cannot agree with this.  I opened
https://reviews.llvm.org/D101245 (Apr 24) for review and the removal was
my second attempt after I realized that it is non-trivial to address
potential miscompilation issues.

>This is a spec optimization, and while that may not be important to Google, that is important to many people who use LLVM.  Can you please revert this patch and discuss if further?

https://bugs.llvm.org/show_bug.cgi?id=50027 was reported by Loris Reiff,
who is not associated with Google. I was helping fixing the issue as an
LLVM community member. The bug reported by Loris could cause an internal
compiler crash but I realized we could have miscompilation issues.


I added abort() in PerformHeapAllocSRoA() and tested an internal spec2006 - no file caused a clang crash.
So this optimization isn't triggered in spec2006.

Arthur Eubanks confirmed this optimization did not fire when bootstrapping clang.

I just checked llvm-test-suite as well - not triggered.

Given that this optimization was not triggered in any of
llvm-project/spec2006/llvm-test-suite and the optimization could cause
miscompilation issues, I'd prefer we don't revert the patch.
Once a regression is found, we can keep investigating.
_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210518/d067db99/attachment.html>