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

Renato Golin renato.golin at linaro.org
Tue Sep 16 07:41:39 PDT 2014


On 16 September 2014 11:29, Moritz Roth <moritz.roth11 at gmail.com> wrote:
> unfortunately there is only ADDS (immediate) in Thumb1, so we have no choice
> :(

Dang, I keep getting confused about that on ARM ARM. I wish we had
them separate by ISA... :(


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

Makes sense.


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

This seems a bit arbitrary... Though, I'm not sure how you could
detect it otherwise.


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

I'm ok with this going in like that, but would be good to add a test
for it when the next patch lands. Would also be good to add a comment
to that effect on the commit message, so that people know why the
commit has no tests.

cheers,
--renato



More information about the llvm-commits mailing list