[PATCH] ARM + Thumb Segmented Stacks (v2)

[Alex Crichton]

> After the "push {lr}" (line 1872-ish), you only need to emit a CFI for the
> lr. The offsets are all relative to the CFA, so the records for r4 and r5 do
> not need to be updated, and the offset for sp will be -12.

Aha! That makes sense.

> Also, after the "pop {r4, r5}" at the beginning of the rest of the function
> (line 1957-ish), you need to emit .cfi_same_value for r4 and r5, to let the
> debugger know that the previous stack frame's values of r4 and r5 are now
> found in the actual registers, not on the stack. This is important, because
> the values in the caller's stack frame will continue to be wrong while the
> function body is executing, which will be visible to the user.

Thanks for the tip! I had to add support of emission for the
cfi_same_value directive in AsmPrinterDwarf.cpp, but it wasn't very
complicated.

>> I've attempted to implement (5), but if I chose the wrong opcodes,
>> just let me know.
>
> Most of them look correct, but the "pop {lr}" instruction is now using
> ARM::tPOP, which corresponds to encoding T1, which can only load into
> registers r0-r7 and pc. (The reason of the difference between the Thumb1
> push and pop instructions is that it can be used for efficient function
> epilogues, popping the lr straight back into the pc.) I see two options:
> * Use encoding T3, which can be any register except for sp. This is a
> Thumb2-only instruction, so this code could not run on Thumb1-only
> processors. I think this is roughly what you had before.
> * Use POP encoding T1 to pop into r4, followed by MOV encoding T1 to move
> from r4 to lr. Both of these instructions are available on Thumb1.
>
> Since Thumb1FrameLowering inherits from ARMFrameLowering, your code is
> currently being run for Thumb1, but emitting code which is not valid Thumb1.
> Because of this, you will need to either ensure you only use Thumb1
> instructions when generating code for Thumb1 or Thumb2, or add an
> implementation of adjustForSegmentedStacks to Thumb1FrameLowering. It is
> fine if this implementation just throws report_fatal_error for now, if you
> don't want to support Thumb1 yet.

If this is the only instruction in question, I think it's easy enough
to support both. I switched Thumb1 to using "pop {r4}; mov lr, r4",
and I added a "pop {lr}" for Thumb2, but the only instruction I found
was t2LDMIA_UPD. You expressed size concerns about this previously,
but I was unable to find opcodes like t2POP or t3POP. Is there one for
the instruction you were mentioning?

I've also added a thumb2-specific test (just a small one) to make sure
the code path is hit. I also accidentally left out a test case for ARM
in the previous patches which I included in this patch (sorry about
that!)

>> According to the original author [1], apparently some Thumb units
>> don't have the coprocessor that the ARM version uses. The location of
>> the stack limit seems like it can be all over the place and is pretty
>> dependent to the platform that it's running on, so I'm not quite sure
>> what the best way is to reconcile all these without adding a zillion
>> configuration options.
>
> This may not be too important an issue at the moment, as rust does not
> support thumb (as far as I can tell, [2] does not contain thumb and the arm
> directory does not have any thumb assembly). Do you know of anyone using
> this for thumb, and what platform they are targeting? However, switching
> based on the instruction set (or any other hardware feature) is really not
> the right way to do this, as this is a platform decision.

For now, we have a contributor who is exploring Thumb, but I think
right now they're predominately working without the standard library
and/or just having a dummy __morestack stub. They haven't upstreamed
their work just yet.

I am unaware of what platforms they're targeting, I'd have to get more
information from them. Perhaps this could continue to use the
coprocessor for the stack limit, and further patches could tweak it
for various platforms?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arm.patch
Type: application/octet-stream
Size: 44613 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140328/78da00bb/attachment.obj>