[Thumb] Don't materialize a new base register when CPSR is live

[Moritz Roth]

Hi Renato,

unfortunately there is only ADDS (immediate) in Thumb1, so we have no
choice :(

I realised this morning that for this to work properly, the ADDS inserted
to materialize the new base needs a <def,dead> of the CPSR (instead of just
a def), so I added that to the patch. I also increased the neighbourhood
size for computeRegisterLiveness slightly (10->15), as it turned out that
quite often, CPSR liveness was undecidable in the default neighbourhood.

And of course good point about the redundant comparison, I've removed that.

Regarding the tests, you're right - I found this while working on making
the load/store merging more aggressive (currently, we only do it if it's
obviously safe) - the register allocator inserted some spills between a CMP
and Bcc which could be merged, but it isn't possible to safely materialize
the base+offset there. Unfortunately, with the algorithm that's currently
checked in, the STRs in that case aren't merged at all, and it's somewhat
dependent on register allocation...

Cheers
Moritz

On 12 September 2014 23:15, Renato Golin <renato.golin at linaro.org> wrote:

> Hi Moritz,
>
> Why do you need to use ADDS there? Wouldn't that problem be gone if
> you used a simple ADD?
>
> Also, this comparison is redundant:
>
> +    if (isThumb1 && !SafeToClobberCPSR)
> +      return false;
>
> could be just
>
> +    if (!SafeToClobberCPSR)
> +      return false;
>
> though, keeping the comment is probably good.
>
> I suppose reducing the case where you found it proved harder than usual?
>
> cheers,
> --renato
>
> On 12 September 2014 17:44, Moritz Roth <Moritz.Roth at arm.com> wrote:
> > Hi Renato,
> >
> >
> >
> > This patch is a small bugfix for the load/store optimizer. If the CPSR is
> > live, we can’t safely materialize a base register + offset in Thumb-1.
> >
> > There is no test case because I haven’t actually seen this cause
> problems in
> > real code – but theoretically, it’s possible to get spill code inserted
> > between e.g. a CMP + Bcc, which would then be a candidate for merging.
> >
> >
> >
> > OK to commit?
> >
> >
> >
> > Cheers
> >
> > Moritz
> >
> >
> > -- 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-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140916/bb635ad8/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ARM-Load-store-optimizer-Don-t-materialize-a-new-bas.patch
Type: text/x-patch
Size: 5973 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140916/bb635ad8/attachment.bin>