[PATCH] Added a new register class for Thumb PC-rel loads

[Daniel Stewart]

Actually in this case, the add between the adr and mov is exactly what is
needed to trip the mov pc, rx type of jump table implementation. What is
happening behind the scenes is that initially a t2LEApcrelJT instruction is
placed in the IR code. It is this instruction that has the restriction of
not using lr, and this instruction that is getting translated into the mov
pc, rx.

LLVM then (in ARMConstantIslandPass.cpp:optimizeThumb2JumpTables) tries to
reduce the mov pc,rx instruction into a tbb instruction. Most of the time it
can do it, especially in straightforward cases of a switch statement.
However, in more complicated cases it cannot, for the very reason you've
mentioned. In these cases the mov pc, rx is kept and no tbb translation is
done. 

It is these oddball cases, where LLVM decides it wants to use the lr for the
index & can't reduce to a tbb instruction, that it produces mov pc, lr. It
is difficult to reproduce, especially with the changing code base, because
the reasons it chooses which particular registers for what job is affected
by so many things. 

In looking at the test case again on both a patched and unpatched version of
LLVM (sync'ed today), you are correct that in both cases the register it
decides to use is the same. I can continue to see if I can produce a test
case that shows the issue. The case I originally found the issue in was on a
slightly different code base than what is available now in the community
(polly being the biggest differentiator). And that particular example does
not produce the same register usage due to the differences in polly.

If I cannot find an example that produces mov pc, lr, I am fine with leaving
out the patch, as I can completely understand the need to have tests to show
the effects of & test the patch. 

Daniel

-----Original Message-----
From: Tilmann Scheller [mailto:tscheller at apple.com] 
Sent: Thursday, September 05, 2013 7:54 AM
To: Daniel Stewart
Cc: Commits
Subject: Re: [PATCH] Added a new register class for Thumb PC-rel loads

Hmm, thinking more about this, I think this is actually not what you really
want.

I noticed two things:
(1) The test case produces the same code with and without the patch, so it's
actually not triggering the added code.
(2) I think the constraint is actually in the wrong place. Constraining the
destination register of the ADR doesn't guarantee you that the final
register used by the MOV will be different from LR.

This is the relevant code in the test case:

        adr.w   r3, LJTI0_0_0
        adds    r1, #4
        movs    r2, #0
        movs    r4, #0
        add.w   r5, r3, r0, lsl #2
                                        @ implicit-def: R3
        mov     pc, r5

As you can see there is an ADD between the ADR and the MOV and the jump
target ends up in r5 rather than r3. Nothing prevents the register allocator
from choosing LR instead of r5.

I'm not familiar with the jump table code, so I can't really say what's the
easiest way to achieve what you want. 

Anyone else who can comment on that?

I'll revert the patch for now.

Regards,

Tilmann


On Sep 5, 2013, at 1:20 PM, Tilmann Scheller <tscheller at apple.com> wrote:

> Yeah, that's what I was thinking too.
> 
> I changed the commit message a bit, since this is actually using the ADR
instruction (not a load) and committed in in 190043.
> 
> Thanks,
> 
> Tilmann
> 
> On Sep 4, 2013, at 10:49 PM, Daniel Stewart <stewartd at codeaurora.org>
wrote:
> 
>> No, not an easy test case to reduce at all. This is actually as reduced
as I could get it.
>> 
>> Daniel
>> 
>> From: Renato Golin [mailto:renato.golin at linaro.org] 
>> Sent: Wednesday, September 04, 2013 3:43 PM
>> To: Tilmann Scheller
>> Cc: Daniel Stewart; Commits
>> Subject: Re: [PATCH] Added a new register class for Thumb PC-rel loads
>> 
>> LGTM.
>> 
>> 
>> On 4 September 2013 20:38, Tilmann Scheller <tscheller at apple.com> wrote:
>> I assume there's not much you can do to reduce your test case even
further, right?
>> 
>> That's not an easy case to reduce, unfortunately.
>> 
>> cheers,
>> --renato
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits