[PATCH, GCC, AARCH64, 3/6] Restrict indirect tail calls to x16 and x17

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH, GCC, AARCH64, 3/6] Restrict indirect tail calls to x16 and x17

Sudakshina Das-2
Hi

This patch is part of a series that enables ARMv8.5-A in GCC and
adds Branch Target Identification Mechanism.
(https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools)

This patch changes the registers that are allowed for indirect tail
calls. We are choosing to restrict these to only x16 or x17.

Indirect tail calls are special in a way that they convert a call
statement (BLR instruction) to a jump statement (BR instruction). For
the best possible use of Branch Target Identification Mechanism, we
would like to place a "BTI C" (call) at the beginning of the function
which is only compatible with BLRs and BR X16/X17. In order to make
indirect tail calls compatible with this scenario, we are restricting
the TAILCALL_ADDR_REGS.

In order to use x16/x17 for this purpose, we also had to change the use
of these registers in the epilogue/prologue handling. For this purpose
we are now using x12 and x13 named as EP0_REGNUM and EP1_REGNUM as
scratch registers for epilogue and prologue.

Bootstrapped and regression tested with aarch64-none-linux-gnu. Updated
test. Ran Spec2017 and no performance hit.

Is this ok for trunk?

Thanks
Sudi


*** gcc/ChangeLog***

2018-xx-xx  Sudakshina Das  <[hidden email]>

          * config/aarch64/aarch64.c (aarch64_expand_prologue): Use new
          epilogue/prologue scratch registers EP0_REGNUM and EP1_REGNUM.
          (aarch64_expand_epilogue): Likewise.
          (aarch64_output_mi_thunk): Likewise
          * config/aarch64/aarch64.h (REG_CLASS_CONTENTS): Change
        TAILCALL_ADDR_REGS
          to x16 and x17.
          * config/aarch64/aarch64.md: Define EP0_REGNUM and EP1_REGNUM.

*** gcc/testsuite/ChangeLog ***

2018-xx-xx  Sudakshina Das  <[hidden email]>

          * gcc.target/aarch64/test_frame_17.c: Update to check for
        EP0_REGNUM instead of IP0_REGNUM and add test case.


rb9885.patch (10K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH, GCC, AARCH64, 3/6] Restrict indirect tail calls to x16 and x17

Sudakshina Das-2
Hi

On 02/11/18 18:37, Sudakshina Das wrote:

> Hi
>
> This patch is part of a series that enables ARMv8.5-A in GCC and
> adds Branch Target Identification Mechanism.
> (https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools)
>
> This patch changes the registers that are allowed for indirect tail
> calls. We are choosing to restrict these to only x16 or x17.
>
> Indirect tail calls are special in a way that they convert a call
> statement (BLR instruction) to a jump statement (BR instruction). For
> the best possible use of Branch Target Identification Mechanism, we
> would like to place a "BTI C" (call) at the beginning of the function
> which is only compatible with BLRs and BR X16/X17. In order to make
> indirect tail calls compatible with this scenario, we are restricting
> the TAILCALL_ADDR_REGS.
>
> In order to use x16/x17 for this purpose, we also had to change the use
> of these registers in the epilogue/prologue handling. For this purpose
> we are now using x12 and x13 named as EP0_REGNUM and EP1_REGNUM as
> scratch registers for epilogue and prologue.
>
> Bootstrapped and regression tested with aarch64-none-linux-gnu. Updated
> test. Ran Spec2017 and no performance hit.
>
> Is this ok for trunk?
>
> Thanks
> Sudi
>
>
> *** gcc/ChangeLog***
>
> 2018-xx-xx  Sudakshina Das  <[hidden email]>
>
>            * config/aarch64/aarch64.c (aarch64_expand_prologue): Use new
>            epilogue/prologue scratch registers EP0_REGNUM and EP1_REGNUM.
>            (aarch64_expand_epilogue): Likewise.
>            (aarch64_output_mi_thunk): Likewise
>            * config/aarch64/aarch64.h (REG_CLASS_CONTENTS): Change
> TAILCALL_ADDR_REGS
>            to x16 and x17.
>            * config/aarch64/aarch64.md: Define EP0_REGNUM and EP1_REGNUM.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2018-xx-xx  Sudakshina Das  <[hidden email]>
>
>            * gcc.target/aarch64/test_frame_17.c: Update to check for
> EP0_REGNUM instead of IP0_REGNUM and add test case.
>
I have edited the patch to take out a change that was not needed as part
of this patch in aarch64_expand_epilogue. The only change now happening
there is as mentioned in the ChangeLog to replace the uses of IP0/IP1.
ChangeLog still applies.

Thanks
Sudi

rb9885.patch (9K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH, GCC, AARCH64, 3/6] Restrict indirect tail calls to x16 and x17

James Greenhalgh-2
On Thu, Nov 29, 2018 at 10:56:46AM -0600, Sudakshina Das wrote:

> Hi
>
> On 02/11/18 18:37, Sudakshina Das wrote:
> > Hi
> >
> > This patch is part of a series that enables ARMv8.5-A in GCC and
> > adds Branch Target Identification Mechanism.
> > (https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools)
> >
> > This patch changes the registers that are allowed for indirect tail
> > calls. We are choosing to restrict these to only x16 or x17.
> >
> > Indirect tail calls are special in a way that they convert a call
> > statement (BLR instruction) to a jump statement (BR instruction). For
> > the best possible use of Branch Target Identification Mechanism, we
> > would like to place a "BTI C" (call) at the beginning of the function
> > which is only compatible with BLRs and BR X16/X17. In order to make
> > indirect tail calls compatible with this scenario, we are restricting
> > the TAILCALL_ADDR_REGS.
> >
> > In order to use x16/x17 for this purpose, we also had to change the use
> > of these registers in the epilogue/prologue handling. For this purpose
> > we are now using x12 and x13 named as EP0_REGNUM and EP1_REGNUM as
> > scratch registers for epilogue and prologue.
> >
> > Bootstrapped and regression tested with aarch64-none-linux-gnu. Updated
> > test. Ran Spec2017 and no performance hit.
> >
> > Is this ok for trunk?

While this isn't strictly needed outside of compilation for targets with BTI
protection enabled, I can well appreciate the simplification in our backend
code to avoid special cases in these areas.

I don't forsee a high likelihood of performance issues from this patch,
but please do keep an eye out for any reports as we move through stage 3.

This is OK for trunk.

Thanks,
James


> >
> > Thanks
> > Sudi
> >
> >
> > *** gcc/ChangeLog***
> >
> > 2018-xx-xx  Sudakshina Das  <[hidden email]>
> >
> >            * config/aarch64/aarch64.c (aarch64_expand_prologue): Use new
> >            epilogue/prologue scratch registers EP0_REGNUM and EP1_REGNUM.
> >            (aarch64_expand_epilogue): Likewise.
> >            (aarch64_output_mi_thunk): Likewise
> >            * config/aarch64/aarch64.h (REG_CLASS_CONTENTS): Change
> > TAILCALL_ADDR_REGS
> >            to x16 and x17.
> >            * config/aarch64/aarch64.md: Define EP0_REGNUM and EP1_REGNUM.
> >
> > *** gcc/testsuite/ChangeLog ***
> >
> > 2018-xx-xx  Sudakshina Das  <[hidden email]>
> >
> >            * gcc.target/aarch64/test_frame_17.c: Update to check for
> > EP0_REGNUM instead of IP0_REGNUM and add test case.
> >
> I have edited the patch to take out a change that was not needed as part
> of this patch in aarch64_expand_epilogue. The only change now happening
> there is as mentioned in the ChangeLog to replace the uses of IP0/IP1.
> ChangeLog still applies.