[PATCH] ARM + Thumb Segmented Stacks (v2)
Oliver Stannard
oliver.stannard at arm.com
Wed Apr 2 09:18:24 PDT 2014
Committed revision 205430.
Oliver
> -----Original Message-----
> From: alexc605 at gmail.com [mailto:alexc605 at gmail.com] On Behalf Of Alex
> Crichton
> Sent: 02 April 2014 16:27
> To: Oliver Stannard
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH] ARM + Thumb Segmented Stacks (v2)
>
> I do not have commit access, so if you could commit for me, it would
> be much appreciated!
>
> Thanks again for all your help!
>
> On Wed, Apr 2, 2014 at 6:20 AM, Oliver Stannard
> <Oliver.Stannard at arm.com> wrote:
> > The patch looks good to me now. Do you have commit access? If not, I
> can commit it for you.
> >
> > Oliver
> >
> >> -----Original Message-----
> >> From: alexc605 at gmail.com [mailto:alexc605 at gmail.com] On Behalf Of
> Alex
> >> Crichton
> >> Sent: 31 March 2014 18:20
> >> To: Oliver Stannard
> >> Cc: llvm-commits at cs.uiuc.edu
> >> Subject: Re: [PATCH] ARM + Thumb Segmented Stacks (v2)
> >>
> >> Thanks again so much for your help and bearing with me!
> >>
> >> I've updated with the t2LDR_POST opcode, using the coprocessor on
> >> Thumb2 (but not Thumb1), tweaking the report_fatal_error condition
> >> (just removing the "!Thumb" clause), and formatted such that no more
> >> lines are over 80 chars.
> >>
> >> On Mon, Mar 31, 2014 at 7:01 AM, Oliver Stannard
> >> <Oliver.Stannard at arm.com> wrote:
> >> >> -----Original Message-----
> >> >> >> 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?
> >> >
> >> > Sorry, I've been using the ARM encoding names, LLVM has different
> >> names for the encodings. I think you want ARM::t2LDR_POST, there are
> >> example of its use at ARMFrameLowering.cpp:{1192,881}. t2LDMIA_UPD
> >> emits the invalid encoding of LDM with one register. You can use
> llvm-
> >> mc to find out the LLVM name of the instruction that you want like
> >> this:
> >> >
> >> > $ llvm-mc -triple thumbv7-none-eabi -assemble -show-inst <<< 'pop
> >> {lr}'
> >> > .text
> >> > ldr lr, [sp], #4 @ <MCInst #2456 t2LDR_POST
> >> > @ <MCOperand Reg:10>
> >> > @ <MCOperand Reg:12>
> >> > @ <MCOperand Reg:12>
> >> > @ <MCOperand Imm:4>
> >> > @ <MCOperand Imm:14>
> >> > @ <MCOperand Reg:0>>
> >> >
> >> >> 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?
> >> >
> >> > Yes, it seems that the best option for now is to use the
> coprocessor
> >> for Thumb2 and ARM, and the STACK_LIMIT symbol for Thumb1. It looks
> >> like the coprocessor register you are using is part of the Virtual
> >> Memory System Architecture (VMSA) extension, which is only available
> >> for A-class processors. However, as far as I am aware, Linux and
> >> Android require an MMU (except for thinks like uCLinux), so I think
> we
> >> can assume the existence of that co-processor if the platform is
> Linux
> >> or Android.
> >> >
> >> > 8)
> >> >> if (!Thumb && !ST->isTargetAndroid() && !ST->isTargetLinux())
> >> >> report_fatal_error("Segmented stacks not supported on this
> >> platfrom.");
> >> >
> >> > I think you should check this condition, it looks like it will not
> >> trigger for "thumb and not (linux or android)". Also the comment
> says
> >> you do not support Thumb1, but you do now.
> >> >
> >> > 9)
> >> > There are a few long lines (>80 chars), clang-format can fix these
> >> and other coding style issues for you.
> >> >
> >> > Oliver
> >> >
> >> > -- IMPORTANT NOTICE: The contents of this email and any
> attachments
> >> are confidential and may also be privileged. If you are not the
> >> intended recipient, please notify the sender immediately and do not
> >> disclose the contents to any other person, use it for any purpose,
> or
> >> store or copy the information in any medium. Thank you.
> >> >
> >> > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1
> 9NJ,
> >> Registered in England & Wales, Company No: 2557590
> >> > ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge
> CB1
> >> 9NJ, Registered in England & Wales, Company No: 2548782
> >> >
> >
> > -- IMPORTANT NOTICE: The contents of this email and any attachments
> are confidential and may also be privileged. If you are not the
> intended recipient, please notify the sender immediately and do not
> disclose the contents to any other person, use it for any purpose, or
> store or copy the information in any medium. Thank you.
> >
> > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
> Registered in England & Wales, Company No: 2557590
> > ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1
> 9NJ, Registered in England & Wales, Company No: 2548782
> >
More information about the llvm-commits
mailing list