[Thumb1] Fix and re-enable the load/store optimization pass

Moritz Roth Moritz.Roth at arm.com
Fri Aug 15 10:09:48 PDT 2014


Hi Renato,

thanks for the review! Committed as r215728 and r215729.

Cheers
Moritz

-----Original Message-----
From: Renato Golin [mailto:renato.golin at linaro.org]
Sent: 14 August 2014 11:15
To: Moritz Roth
Cc: LLVM Commits; dpeixott at quicinc.com; James Molloy; sgundapa
Subject: Re: [Thumb1] Fix and re-enable the load/store optimization pass

Hi Moritz,

LGTM, thanks!

--renato

On 14 August 2014 11:08, Moritz Roth <moritz.roth at arm.com> wrote:
> Hi Renato,
>
>>> 0002: Make the merging more conservative on Thumb1.
>>
>> I just don't get this one. This should be disabled in Thumb1, right?
>> Why bother changing the behaviour and not re-enabling it on the same
>> patch?
>
> Fair point, I've merged the last two patches as you suggested.
>
>> Did you change getMaxInlineSizeThreshold() because you're not doing
>> the LDM on two registers on T1 any more? You plan on doing the full
>> fold in the future even on T1?
>>
>> Either way, a comment / fixme to that effect would be good.
>
> Yes, the intention was to avoid generating long chains of loads/stores since
> the algorithm in its current form can't properly merge all of them.
> However, upon looking at some generated code again I've decided to scrap this.
> Even with larger memcpys approaching the cut-off there is still a good amount
> of LDM/STM folding going on. And yes, I'm planning on submitting a follow-up
> patch to allow memcpys to be fully folded even on T1.
>
>>> 0003: Re-enable the pass, and add test cases for the previous two patches.
>>> There are three new regression tests created from simple test programs that
>>> were miscompiled previously, and I’ve added –verify-machineinstrs to each of
>>> the tests as well.
>>
>> If I got it right, you shouldn't have SUBS after some LDMs, so would
>> be good to CHECK-NOT them on the appropriate tests.
>
> Yes, that's right. I've made sure the tests where the pass would previously
> insert a SUBS have such a check now. Updated patches are attached.
>
> Cheers
> Moritz
>
>> Thanks!
>> --renato


-- 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




More information about the llvm-commits mailing list