<div dir="ltr">Hi Renato,<div><br></div><div>unfortunately there is only ADDS (immediate) in Thumb1, so we have no choice :(</div><div><br></div><div>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.</div><div><br></div><div>And of course good point about the redundant comparison, I've removed that.</div><div><br></div><div>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...</div><div><br></div><div>Cheers</div><div>Moritz</div></div><div class="gmail_extra"><br><div class="gmail_quote">On 12 September 2014 23:15, Renato Golin <span dir="ltr"><<a href="mailto:renato.golin@linaro.org" target="_blank">renato.golin@linaro.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Moritz,<br>
<br>
Why do you need to use ADDS there? Wouldn't that problem be gone if<br>
you used a simple ADD?<br>
<br>
Also, this comparison is redundant:<br>
<br>
+    if (isThumb1 && !SafeToClobberCPSR)<br>
+      return false;<br>
<br>
could be just<br>
<br>
+    if (!SafeToClobberCPSR)<br>
+      return false;<br>
<br>
though, keeping the comment is probably good.<br>
<br>
I suppose reducing the case where you found it proved harder than usual?<br>
<br>
cheers,<br>
--renato<br>
<div><div class="h5"><br>
On 12 September 2014 17:44, Moritz Roth <<a href="mailto:Moritz.Roth@arm.com">Moritz.Roth@arm.com</a>> wrote:<br>
> Hi Renato,<br>
><br>
><br>
><br>
> This patch is a small bugfix for the load/store optimizer. If the CPSR is<br>
> live, we can’t safely materialize a base register + offset in Thumb-1.<br>
><br>
> There is no test case because I haven’t actually seen this cause problems in<br>
> real code – but theoretically, it’s possible to get spill code inserted<br>
> between e.g. a CMP + Bcc, which would then be a candidate for merging.<br>
><br>
><br>
><br>
> OK to commit?<br>
><br>
><br>
><br>
> Cheers<br>
><br>
> Moritz<br>
><br>
><br>
> -- IMPORTANT NOTICE: The contents of this email and any attachments are<br>
> confidential and may also be privileged. If you are not the intended<br>
> recipient, please notify the sender immediately and do not disclose the<br>
> contents to any other person, use it for any purpose, or store or copy the<br>
> information in any medium. Thank you.<br>
><br>
> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,<br>
> Registered in England & Wales, Company No: 2557590<br>
> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,<br>
> Registered in England & Wales, Company No: 2548782<br>
<br>
</div></div>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>