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

[Tilmann Scheller]

Hi Daniel,

On Sep 5, 2013, at 4:10 PM, Daniel Stewart <stewartd at codeaurora.org> wrote:

> 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.
Correct, with your patch t2LEApcrelJT has the proper constraint to not use LR for the destination register.

My understanding is that the t2LEApcrelJT gets expanded to an ADR instruction eventually. However, the MOV seems to be completely independent of the t2LEApcrelJT, thus there’s no way the constraint on t2LEApcrelJT helps to constrain the source register of the MOV (correct me if I’m wrong here).

While the register allocator might assign registers differently with your patch (since after all we have an additional constraint on t2LEApcrelJT), I’m arguing that it can still get away with assigning LR to the source register of the MOV (since there’s no constraint which prevents this). In other words, the patch cures the symptoms but not the root cause :)

> 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. 
Yeah, I can imagine :)

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

Regards,

Tilmann

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130905/47896cb1/attachment.html>