[LLVMdev] GlobalsModRef (and thus LTO) is completely broken

Fri Jul 17 08:10:20 PDT 2015

Can you say what Benchmark or give a test case so we understand the nature
of the regression? As Gerolf said, that will be important to understand
what is best to do.

On Fri, Jul 17, 2015, 06:43 Evgeny Astigeevich <Evgeny.Astigeevich at arm.com>
wrote:

>  Yes, the regression is stable. I double checked this. A full benchmark
> run consists of at least 10 sub-runs to validate the score.
>
> I also checked if there were regressions of this benchmark across
> different ARM hardware versions. I found all regressions of this benchmark
> were in range 1.6%-2%.
>
>
>
> Kind regards,
>
> Evgeny Astigeevich
>
>
>
> *From:* Chandler Carruth [mailto:chandlerc at gmail.com]
> *Sent:* 17 July 2015 07:52
> *To:* Evgeny Astigeevich; Chandler Carruth
> *Cc:* LLVM Developers Mailing List; Michael Zolotukhin
>
>
> *Subject:* Re: [LLVMdev] GlobalsModRef (and thus LTO) is completely broken
>
>
>
> Hey, thanks for benchmarking.
>
>
>
> How stable is the 2% regression?
>
>
>
> Michael ran some benchmarks with GlobalsModRef completely disabled and the
> only differences were in the noise. This was a complete spec2k6 run along
> with some others. Based on the number of benchmarks run there, I'm going to
> go ahead and submit these patches, but if you can clarify the impact here,
> we can look at potentially some other tradeoff. I'm not particularly set on
> one set of defaults, etc, I just don't want to keep patches held up based
> on that. We can flip the default back and forth as new data arrives.
>
>
>
> On Thu, Jul 16, 2015 at 12:23 PM Evgeny Astigeevich <
> evgeny.astigeevich at arm.com> wrote:
>
>  Hi Chandler,
>
>
>
> I ran couple benchmarks with LTO turned on and your patches on ARM
> hardware.
>
> There were no performance degradation of one benchmark and 2% slowdown of
> another benchmark.
>
>
>
> Kind regards,
>
> Evgeny Astigeevich
>
>
>
>
>
> *From:* llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] *On
> Behalf Of *Evgeny Astigeevich
> *Sent:* 15 July 2015 15:12
>
>
> *To:* 'Chandler Carruth'; Gerolf Hoflehner
> *Cc:* LLVM Developers Mailing List
> *Subject:* Re: [LLVMdev] GlobalsModRef (and thus LTO) is completely broken
>
>
>
> Hi Chandler,
>
>
>
> I would like to run some benchmarks on ARM hardware and to look at impact
> of your patches on LTO.
>
>
>
> Kind regards,
>
> Evgeny Astigeevich
>
>
>
> *From:* llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu
> <llvmdev-bounces at cs.uiuc.edu>] *On Behalf Of *Chandler Carruth
>
>
> *Sent:* 15 July 2015 10:45
> *To:* Chandler Carruth; Gerolf Hoflehner
> *Cc:* LLVM Developers Mailing List
> *Subject:* Re: [LLVMdev] GlobalsModRef (and thus LTO) is completely broken
>
>
>
> I've fixed the obvious bugs I spotted in r242281. These should be pure
> correctness improvements.
>
>
>
> I've sent the two patches I'm imagining to address the core issue here:
>
> http://reviews.llvm.org/D11213
> <https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11213&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=Mfk2qtn1LTDThVkh6-oGglNfMADXfJdty4_bhmuhMHA&m=TDTMPyN-BRKio_S9jhFxP6vHW7gAN3F73DTvS3M46go&s=TjLIFmohippEitr9aFYFcADeLJeQ-z2E_LH0fpsL38Q&e=>
>
> http://reviews.llvm.org/D11214
> <https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11214&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=Mfk2qtn1LTDThVkh6-oGglNfMADXfJdty4_bhmuhMHA&m=TDTMPyN-BRKio_S9jhFxP6vHW7gAN3F73DTvS3M46go&s=z8Wdu7ZruKrV5dqBOmaBxWe1aaiDWtf4it9ZI1dVweQ&e=>
>
>
>
> Currently, I have the unsafe alias results disabled by default, but with a
> flag that can re-enable them if needed. I don't feel really strongly about
> which way the default is set -- but that may be because I don't have lots
> of users relying on LTO. I'll let others indicate which way they would be
> most comfortable.
>
>
>
> Some IRC conversations indicated that early benchmark results with GMR
> completely disabled weren't showing really significant swings, so maybe
> this relatively small reduction in power of GMR won't be too problematic
> for folks. Either way, I'm open to the different approaches. It's D11214
> that I care a lot about. =]
>
>
>
>
>
> Thanks for all the thoughts here!
>
> -Chandler
>
>
>
> On Tue, Jul 14, 2015 at 11:25 PM Chandler Carruth <chandlerc at gmail.com>
> wrote:
>
>  Replying here, but several of the questions raised boil down to
> "couldn't you make the usage of GetUnderlyingObject conservatively
> correct?". I'll try and address that.
>
>
>
> I think this *is* the right approach, but I think it is very hard to do
> without effectively disabling this part of GlobalsModRef. That is, the easy
> ways are likely to fire very frequently IMO.
>
>
>
> The core idea is to detect a "no information" state coming out of
> GetUnderlyingObject (likely be providing a custom version just for
> GlobalsModRef and tailored to its use cases). This is particularly
> effective at avoiding the problems with the recursion limit. But let's look
> at what cases we *wouldn't* return that. Here are the cases I see when I
> thought about this last night with Hal, roughly in descending likelihood I
> would guess:
>
>
>
> 1) We detect some global or an alloca. In that case, even BasicAA would be
> sufficient to provide no-alias. GMR shouldn't be relevant.
>
>
>
> 2) We detect a phi, select, or inttoptr, and stop there.
>
>
>
> 3) We detect a load and stop there.
>
>
>
> 4) We detect a return from a function.
>
>
>
> 5) We detect an argument to the function.
>
>
>
> I strongly suspect the vast majority of queries hit #1. That's why BasicAA
> is *so* effective. Both #4 and #5 I think are actually reasonable places
> for GMR to potentially say "no-alias" and provide useful definitive
> information. But I also suspect these are the least common.
>
>
>
> So let's look at #2 and #3 because I think they're interesting. For these,
> I think it is extremely hard to return "no-alias". It seems extremely easy
> for a reasonable and innocuous change to the IR to introduce a phi or a
> select into one side of the GetUnderlyingObject but not the other. If that
> ever happens, we can't return "no-alias" for #2, or we need to add really
> expensive updates. It also seems reasonable (if not as likely) to want
> adding a store and load to the IR to not trigger a miscompile. If it is
> possible for a valid optimization pass to do reg2mem on an SSA value, then
> that could happen to only one side of the paired GetUnderlyingObject and
>  break GMR with #3. If that seems like an unreasonable thing to do,
> consider loop re-rolling or other transformations which may need to take
> things in SSA form at move them out of SSA form. Even if we then try
> immediately to put it back *into* SSA form, before we do that we create a
> point where GMR cannot correctly return no-alias.
>
>
>
> So ultimately, I don't think we want to rely on GMR returning "no-alias"
> for either #2 or #3 because of the challenge of actually updating it in all
> of the circumstances that could break them. That means that *only* #4 and
> #5 are going to return "no-alias" usefully. And even then, function
> inlining and function outlining both break #4 and #5, so you have to
> preclude those transforms while GMR is active. And I have serious doubts
> about these providing enough value to be worth the cost.
>
>
>
>
>
> I think the better way to approach this is the other way around. Rather
> than doing a backwards analysis to see if one location reaches and global
> and the other location doesn't reach a global, I think it would be much
> more effective to re-cast this as a forward analysis that determines all
> the memory locations in a function that come from outside the function, and
> use that to drive the no-alias responses.
>
>
>
>
>
> On Tue, Jul 14, 2015 at 12:12 PM Gerolf Hoflehner <ghoflehner at apple.com>
> wrote:
>
>  I wouldn’t be willing to give up performance for hypothetical issues.
> Please protect all your changes with options. For some of your concerns it
> is probably hard to provide a test case that shows an/the actual issue.
>
>
>
> I certainly agree that it will be very hard to provide a test case and
> extremely rare to see this in the wild for most of these issues. As long as
> I can remove the problematic update API we currently have (which as its an
> API change can't really be put behind flags), I'm happy to have flags
> control whether or not GMR uses the unsound / stale information to try to
> answer alias queries. Do you have any opinion about what the default value
> of the flags should be?
>
>
>
> I'll go ahead and prepare the patches, as it seems like we're all ending
> up in the same position, and just wondering about the precise tradeoffs we
> want to settle on.
>
>
>
>   _______________________________________________
> LLVM Developers mailing list
>
>    LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>     _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
> -- IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
>
> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
> Registered in England & Wales, Company No: 2557590
> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
> Registered in England & Wales, Company No: 2548782
>  _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150717/62f08da3/attachment.html>