| Message ID | e2a1d71e-8643-ca06-099f-cca523237c09@acm.org |
|---|---|
| State | New |
| Headers | show |
Nathan, This patch will do what you intend it to do. However, I am not sure in part related to VxWorks. The logic behind this patch is that -mno-pic-data-is-text-relative should enable -msingle-pic-base because otherwise it will be useless. The logic itself is orthogonal to OS. So I am not convinced the 'else if' shouldn't be just 'if'. It should not change VxWorks behaviour if VxWorks enables -msingle-pic-base explicitly. Or otherwise there is at least one use case that -mno-pic-data-is-text-relative can be used without -msingle-pic-base, which breaks the logic that this whole patch stands on. Thanks, Joey > -----Original Message----- > From: Nathan Sidwell [mailto:nathanmsidwell@gmail.com] On Behalf Of > Nathan Sidwell > Sent: 09 May 2016 15:07 > To: Richard Earnshaw; GCC Patches > Cc: Joey Ye > Subject: [ARM] mno-pic-data-is-text-relative & msingle-pic-base > > This patch comes from an off-list conversation between Joey & me. The > context is with RTOSs not all singing & dancing dynamic objects and OSes. > > currently, the documentation for -mno-pic-data-is-text-relative (-mno-PDITR) > says 'Assume that each data segments are relative to text segment at load > time. > Therefore, it permits addressing data using PC-relative operations. > This option is on by default for targets other than VxWorks RTP.' > > However, if you use just this option, you still end up with a pic-register init > sequence that presumes a fixed mapping. That's a surprise. Joey tells me > its expected use is with -msingle-pic-base (-mSPB), which reserves a global > register to point at the (single) GOT. That's what I had expected the -mno- > PDITR option to have implied. > > Apparently there are legitimate reasons one might want the -mno-PDITR > behaviour without -mSPB. I don't know what those are, perhaps Joey could > clarify? > > Anyway, IMHO that is the rare case and the more common case is that one > would want to have -mnoPDITR imply -mSPB. (The reverse probably doesn't > apply.) > > This patch does 3 things. > 1) have -mno-PDITR imply -mSPB, unless one has explictly provided -m[no- > ]SPB. > 2) clarified the -m[no-]PDITR documentation. > 3) Added some testcases -- there didn't appear to be any. > > ok? > > nathan
Joey,
> This patch will do what you intend it to do. However, I am not sure in part related to VxWorks. The logic behind this patch is that -mno-pic-data-is-text-relative should enable -msingle-pic-base because otherwise it will be useless. The logic itself is orthogonal to OS. So I am not convinced the 'else if' shouldn't be just 'if'. It should not change VxWorks behaviour if VxWorks enables -msingle-pic-base explicitly. Or otherwise there is at least one use case that -mno-pic-data-is-text-relative can be used without -msingle-pic-base, which breaks the logic that this whole patch stands on.
VxWorks has two modes of code generation -- kernel and RTP. RTPs don't have a
fixed mapping between code and data (and use special sequence to initialize the
PIC register, using vxworks-specific relocs). Kernel mode doesn't support PIC
code generation -- see config/vxworks.c
So I don't think there's a problem.
nathan
> -----Original Message----- > From: Nathan Sidwell [mailto:nathanmsidwell@gmail.com] On Behalf Of > Nathan Sidwell > Sent: 09 May 2016 18:22 > To: Joey Ye; Richard Earnshaw; GCC Patches > Subject: Re: [ARM] mno-pic-data-is-text-relative & msingle-pic-base > > Joey, > > This patch will do what you intend it to do. However, I am not sure in part > related to VxWorks. The logic behind this patch is that -mno-pic-data-is- > text-relative should enable -msingle-pic-base because otherwise it will be > useless. The logic itself is orthogonal to OS. So I am not convinced the 'else > if' shouldn't be just 'if'. It should not change VxWorks behaviour if VxWorks > enables -msingle-pic-base explicitly. Or otherwise there is at least one use > case that -mno-pic-data-is-text-relative can be used without -msingle-pic- > base, which breaks the logic that this whole patch stands on. > > VxWorks has two modes of code generation -- kernel and RTP. RTPs don't > have a fixed mapping between code and data (and use special sequence to > initialize the PIC register, using vxworks-specific relocs). Kernel mode > doesn't support PIC code generation -- see config/vxworks.c > > So I don't think there's a problem. No other commit. OK for me though I cannot approve it. - Joey
Nathan,
Sorry for jumping in a relatively old thread. I saw this in mailing list
archives during a web search (I wasn't on this mailing list before). I
decided to speak up as I would be an affected party if this patch (or
some similar future patch) gets accepted or worse yet, the feature
involved gets disabled.
> Apparently there are legitimate reasons one might want the -mno-PDITR
behaviour without -mSPB. I don't know what those are, perhaps Joey could
clarify?
Yes, there are some practical use cases that require
-mno-pic-data-is-text-relative (-mno-PDITR) without -msingle-pic-base
(-mSPB).
These are based on two primary principles:
1. In the absence of lazy binding (that is almost always the case in
embedded systems), GOT is practically read-only - it needs to be
modified only during linking by the dynamic linker, after that it can be
considered and marked read-only (e.g. read-only attribute set to be
enforced by some MMU or MPU).
2. If you only need a simple dynamic object model - where you just need
dynamic loading and dynamic linking - but do not need to maintain
multiple data states for the object like you need in a traditional
shared object model, then one instance of GOT per dynamic object is enough.
From #1: GOT is read-only so keeping it with CODE segment is a natural
choice. Now as there is no need to move it to RAM so there is no need
for a mechanism (-mSPB) that would enable controlling the GOT location
independently of CODE segment.
From #2: Only one instance of GOT is required per dynamic object so
there is no need for a mechanism (-mSPB) that would enable switching GOTs.
So when both #1 and #2 are met, you only need
-mno-pic-data-is-text-relative.
Irfan Ahmad
Ramana, I saw some correspondence between you and Nathan on his patch [https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00630.html] (after I sent this email) going in a direction that may eventually result in too tight than necessary coupling between these two switches, as your response hints at: > I am also slightly inclined to go further and error out if someone uses -mno-PDITR with -mno-SPB on the command line, after all as you say -mno-PDITR implies a non-fixed mapping while -mno-SPB implies there is some fixed mapping some where currently in the compiler. I don't see how the twain can meet. That can happen as a follow-up - the current patch is by itself a step improvement. Please see the alternative perspective as described below. Irfan Ahmad [p.s. Sorry about repeat send. I accidentally sent it earlier in HTML format] ----------------------------------------------------------------------------------------------------------- On 05/08/2016 09:56, Irfan Ahmad wrote: Nathan, Sorry for jumping in a relatively old thread. I saw this in mailing list archives during a web search (I wasn't on this mailing list before). I decided to speak up as I would be an affected party if this patch (or some similar future patch) gets accepted or worse yet, the feature involved gets disabled. > Apparently there are legitimate reasons one might want the -mno-PDITR behaviour without -mSPB. I don't know what those are, perhaps Joey could clarify? Yes, there are some practical use cases that require -mno-pic-data-is-text-relative (-mno-PDITR) without -msingle-pic-base (-mSPB). These are based on two primary principles: 1. In the absence of lazy binding (that is almost always the case in embedded systems), GOT is practically read-only - it needs to be modified only during linking by the dynamic linker, after that it can be considered and marked read-only (e.g. read-only attribute set to be enforced by some MMU or MPU). 2. If you only need a simple dynamic object model - where you just need dynamic loading and dynamic linking - but do not need to maintain multiple data states for the object like you need in a traditional shared object model, then one instance of GOT per dynamic object is enough. From #1: GOT is read-only so keeping it with CODE segment is a natural choice. Now as there is no need to move it to RAM so there is no need for a mechanism (-mSPB) that would enable controlling the GOT location independently of CODE segment. From #2: Only one instance of GOT is required per dynamic object so there is no need for a mechanism (-mSPB) that would enable switching GOTs. So when both #1 and #2 are met, you only need -mno-pic-data-is-text-relative. Irfan Ahmad
Irfan, Thanks for the case sharing. It is a persuasive reason not to error out -mno-SPB. Nathan's patch changes default behaviour that -mSPB will be implied when -mno-PDITR is provided. So with this patch your project need to explicitly specify -mno-SPB to make it work as before. IMHO default behaviour should reflect the typical usage. Now I not so sure whether -mSPB should be typically used with -mno-PDITR. Irfan what's your opinion? Thanks, Joey > -----Original Message----- > From: Irfan Ahmad [mailto:h.irfanahmad@gmail.com] > Sent: 05 August 2016 07:06 > To: Ramana Radhakrishnan; Nathan Sidwell; Joey Ye; GCC Patches > Cc: Richard Earnshaw > Subject: Re: [ARM] mno-pic-data-is-text-relative & msingle-pic-base > > Ramana, > > I saw some correspondence between you and Nathan on his patch > [https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00630.html] (after I sent this > email) going in a direction that may eventually result in too tight than necessary > coupling between these two switches, as your response hints at: > > > I am also slightly inclined to go further and error out if someone uses -mno- > PDITR with -mno-SPB on the command line, after all as you say -mno-PDITR > implies a non-fixed mapping while -mno-SPB implies there is some fixed mapping > some where currently in the compiler. I don't see how the twain can meet. That > can happen as a follow-up - the current patch is by itself a step improvement. > > Please see the alternative perspective as described below. > > Irfan Ahmad > [p.s. Sorry about repeat send. I accidentally sent it earlier in HTML format] > -------------------------------------------------------------------------------------------------- > --------- > On 05/08/2016 09:56, Irfan Ahmad wrote: > > Nathan, > > Sorry for jumping in a relatively old thread. I saw this in mailing list archives > during a web search (I wasn't on this mailing list before). I decided to speak up as > I would be an affected party if this patch (or some similar future patch) gets > accepted or worse yet, the feature involved gets disabled. > > > Apparently there are legitimate reasons one might want the -mno-PDITR > behaviour without -mSPB. I don't know what those are, perhaps Joey could > clarify? > > Yes, there are some practical use cases that require -mno-pic-data-is-text- > relative (-mno-PDITR) without -msingle-pic-base (-mSPB). > > These are based on two primary principles: > > 1. In the absence of lazy binding (that is almost always the case in embedded > systems), GOT is practically read-only - it needs to be modified only during > linking by the dynamic linker, after that it can be considered and marked read- > only (e.g. read-only attribute set to be enforced by some MMU or MPU). > > 2. If you only need a simple dynamic object model - where you just need > dynamic loading and dynamic linking - but do not need to maintain multiple data > states for the object like you need in a traditional shared object model, then one > instance of GOT per dynamic object is enough. > > From #1: GOT is read-only so keeping it with CODE segment is a natural choice. > Now as there is no need to move it to RAM so there is no need for a mechanism > (-mSPB) that would enable controlling the GOT location independently of CODE > segment. > > From #2: Only one instance of GOT is required per dynamic object so there is no > need for a mechanism (-mSPB) that would enable switching GOTs. > > So when both #1 and #2 are met, you only need -mno-pic-data-is-text-relative. > > Irfan Ahmad
On 08/05/16 00:56, Irfan Ahmad wrote: > 1. In the absence of lazy binding (that is almost always the case in embedded > systems), GOT is practically read-only - it needs to be modified only during 'practically' as in 'almost', or is it really read only? > linking by the dynamic linker, after that it can be considered and marked > read-only (e.g. read-only attribute set to be enforced by some MMU or MPU). True, but I don't see how that's related to the -mNPDITR flag. (Your statement is about lazy binding). > 2. If you only need a simple dynamic object model - where you just need dynamic > loading and dynamic linking - but do not need to maintain multiple data states > for the object like you need in a traditional shared object model, then one > instance of GOT per dynamic object is enough. Again, correct, but unrelated to the flag. You seem to be claiming that unix-like ABIS require more than one GOT per dynamic object? > From #1: GOT is read-only so keeping it with CODE segment is a natural choice. > Now as there is no need to move it to RAM so there is no need for a mechanism > (-mSPB) that would enable controlling the GOT location independently of CODE > segment. That's correct, but again unrelated to the -mNPDITR flag. > > 2. If you only need a simple dynamic object model - where you just need dynamic loading and dynamic linking - but do not need to maintain multiple data states for the object like you need in a traditional shared object model, then one instance of GOT per dynamic object is enough. What 'multiple data states?' Are you talking about lazy binfing again (unrelated to -mNPDITR), or something else (what?) > So when both #1 and #2 are met, you only need -mno-pic-data-is-text-relative. Your conclusion doesn't follow. You can satisfy them with regular GOT behavior. nathan
Joey, Thanks for your understanding response and sorry for my late reply. I have been badly caught up in work related stuff, lately. As you might already be aware of, -mno-PDITR and -mSPB are primarily targeted for MMU-less targets (or where you do have MMU but do not want to/cannot use address translation (virtual address mapping) for other reasons/constraints). Typically these targets are relatively low-end (to mid-range) and both processing power and RAM can be precious resources. On such targets dynamic linking and loading may be a kind of luxury. So instead of full-blown process model/shared object model, simpler dynamic models are more viable. Typically a simple dynamic model, that allows XIP (execute in-place) execution as well, can be implemented with -mno-PDITR only. -mSPB is needed only when you need to control the location of GOT as well. Now moving GOT independent of TEXT section is normally not required as it can be treated as read-only and so be kept with CODE segment (hence at fixed offset to TEXT). Moreover introducing -mSPB comes at a performance cost and considerable complexity on part of the OS runtime that implements such a dynamic model (as you need to introduce traps and call stubs/trampoline layers to transparently switch the GOT on inter-object call boundaries). So the typical usage of these flags is -mno-PDITR alone. On the other hand, when -mSPB is used, -mno-PDITR would typically also be used, because otherwise your local-scope i.e. static globals would not be placed correctly as compiler normally does not route them through GOT without -mno-PDITR. So -mSPB without -mno-PDITR is usable only if your dynamic objects have no static globals. In short, -mno-PDITR alone is highly useful and should not imply -mSPB. However, if we do want to do something, -mSPB can be made to imply -mno-PDITR as this is the typical usage for -mSPB. So Nathan's patch should have been the other way around: -mSPB should imply -mno-PDITR rather than -mno-PDITR implying -mSPB. Thanks, Irfan Ahmad On Fri, Aug 5, 2016 at 2:12 PM, Joey Ye <joey.ye@foss.arm.com> wrote: > Irfan, > > Thanks for the case sharing. It is a persuasive reason not to error out -mno-SPB. > > Nathan's patch changes default behaviour that -mSPB will be implied when -mno-PDITR is provided. So with this patch your project need to explicitly specify -mno-SPB to make it work as before. IMHO default behaviour should reflect the typical usage. Now I not so sure whether -mSPB should be typically used with -mno-PDITR. Irfan what's your opinion? > > Thanks, > Joey > >> -----Original Message----- >> From: Irfan Ahmad [mailto:h.irfanahmad@gmail.com] >> Sent: 05 August 2016 07:06 >> To: Ramana Radhakrishnan; Nathan Sidwell; Joey Ye; GCC Patches >> Cc: Richard Earnshaw >> Subject: Re: [ARM] mno-pic-data-is-text-relative & msingle-pic-base >> >> Ramana, >> >> I saw some correspondence between you and Nathan on his patch >> [https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00630.html] (after I sent this >> email) going in a direction that may eventually result in too tight than necessary >> coupling between these two switches, as your response hints at: >> >> > I am also slightly inclined to go further and error out if someone uses -mno- >> PDITR with -mno-SPB on the command line, after all as you say -mno-PDITR >> implies a non-fixed mapping while -mno-SPB implies there is some fixed mapping >> some where currently in the compiler. I don't see how the twain can meet. That >> can happen as a follow-up - the current patch is by itself a step improvement. >> >> Please see the alternative perspective as described below. >> >> Irfan Ahmad >> [p.s. Sorry about repeat send. I accidentally sent it earlier in HTML format] >> -------------------------------------------------------------------------------------------------- >> --------- >> On 05/08/2016 09:56, Irfan Ahmad wrote: >> >> Nathan, >> >> Sorry for jumping in a relatively old thread. I saw this in mailing list archives >> during a web search (I wasn't on this mailing list before). I decided to speak up as >> I would be an affected party if this patch (or some similar future patch) gets >> accepted or worse yet, the feature involved gets disabled. >> >> > Apparently there are legitimate reasons one might want the -mno-PDITR >> behaviour without -mSPB. I don't know what those are, perhaps Joey could >> clarify? >> >> Yes, there are some practical use cases that require -mno-pic-data-is-text- >> relative (-mno-PDITR) without -msingle-pic-base (-mSPB). >> >> These are based on two primary principles: >> >> 1. In the absence of lazy binding (that is almost always the case in embedded >> systems), GOT is practically read-only - it needs to be modified only during >> linking by the dynamic linker, after that it can be considered and marked read- >> only (e.g. read-only attribute set to be enforced by some MMU or MPU). >> >> 2. If you only need a simple dynamic object model - where you just need >> dynamic loading and dynamic linking - but do not need to maintain multiple data >> states for the object like you need in a traditional shared object model, then one >> instance of GOT per dynamic object is enough. >> >> From #1: GOT is read-only so keeping it with CODE segment is a natural choice. >> Now as there is no need to move it to RAM so there is no need for a mechanism >> (-mSPB) that would enable controlling the GOT location independently of CODE >> segment. >> >> From #2: Only one instance of GOT is required per dynamic object so there is no >> need for a mechanism (-mSPB) that would enable switching GOTs. >> >> So when both #1 and #2 are met, you only need -mno-pic-data-is-text-relative. >> >> Irfan Ahmad > >
2016-05-09 Nathan Sidwell <nathan@acm.org> gcc/ * config/arm/arm.c (arm_option_override): Set MASK_SINGLE_PIC_BASE when -mno-pic-data-is-text-relative is in effect, by default. * doc/invoke.texi (mpic-data-is-text-relative): Document new behavior and clarify. gcc/testsuite/ * gcc.target/arm/data-rel-1.c: New. * gcc.target/arm/data-rel-2.c: New. * gcc.target/arm/data-rel-3.c: New. Index: config/arm/arm.c =================================================================== --- config/arm/arm.c (revision 235980) +++ config/arm/arm.c (working copy) @@ -3298,6 +3298,20 @@ arm_option_override (void) } } + if (TARGET_VXWORKS_RTP) + { + if (!global_options_set.x_arm_pic_data_is_text_relative) + arm_pic_data_is_text_relative = 0; + } + else if (flag_pic + && !arm_pic_data_is_text_relative + && !(global_options_set.x_target_flags & MASK_SINGLE_PIC_BASE)) + /* When text & data segments don't have a fixed displacement, the + intended use is with a single, read only, pic base register. + Unless the user explicitly requested not to do that, set + it. */ + target_flags |= MASK_SINGLE_PIC_BASE; + /* If stack checking is disabled, we can use r10 as the PIC register, which keeps r9 available. The EABI specifies r9 as the PIC register. */ if (flag_pic && TARGET_SINGLE_PIC_BASE) @@ -3329,10 +3343,6 @@ arm_option_override (void) arm_pic_register = pic_register; } - if (TARGET_VXWORKS_RTP - && !global_options_set.x_arm_pic_data_is_text_relative) - arm_pic_data_is_text_relative = 0; - /* Enable -mfix-cortex-m3-ldrd by default for Cortex-M3 cores. */ if (fix_cm3_ldrd == 2) { Index: doc/invoke.texi =================================================================== --- doc/invoke.texi (revision 235980) +++ doc/invoke.texi (working copy) @@ -14197,9 +14197,12 @@ otherwise the default is @samp{R10}. @item -mpic-data-is-text-relative @opindex mpic-data-is-text-relative -Assume that each data segments are relative to text segment at load time. -Therefore, it permits addressing data using PC-relative operations. -This option is on by default for targets other than VxWorks RTP. +Assume that the displacement between the text and data segments is fixed +at static link time. This permits using PC-relative addressing +operations to access data known to be in the data segment. For +non-VxWorks RTP targets, this option is enabled by default. When +disabled on such targets, it will enable @option{-msingle-pic-base} by +default. @item -mpoke-function-name @opindex mpoke-function-name Index: testsuite/gcc.target/arm/data-rel-1.c =================================================================== --- testsuite/gcc.target/arm/data-rel-1.c (nonexistent) +++ testsuite/gcc.target/arm/data-rel-1.c (working copy) @@ -0,0 +1,12 @@ +/* { dg-options "-fPIC -mno-pic-data-is-text-relative" } */ +/* { dg-final { scan-assembler-not "j-\\(.LPIC" } } */ +/* { dg-final { scan-assembler-not "_GLOBAL_OFFSET_TABLE_-\\(.LPIC" } } */ +/* { dg-final { scan-assembler "j\\(GOT\\)" } } */ +/* { dg-final { scan-assembler "(ldr|mov)\tr\[0-9\]+, \\\[?r9" } } */ + +static int j; + +int *Foo () +{ + return &j; +} Index: testsuite/gcc.target/arm/data-rel-2.c =================================================================== --- testsuite/gcc.target/arm/data-rel-2.c (nonexistent) +++ testsuite/gcc.target/arm/data-rel-2.c (working copy) @@ -0,0 +1,11 @@ +/* { dg-options "-fPIC -mno-pic-data-is-text-relative -mno-single-pic-base" } */ +/* { dg-final { scan-assembler-not "j-\\(.LPIC" } } */ +/* { dg-final { scan-assembler "_GLOBAL_OFFSET_TABLE_-\\(.LPIC" } } */ +/* { dg-final { scan-assembler "j\\(GOT\\)" } } */ + +static int j; + +int *Foo () +{ + return &j; +} Index: testsuite/gcc.target/arm/data-rel-3.c =================================================================== --- testsuite/gcc.target/arm/data-rel-3.c (nonexistent) +++ testsuite/gcc.target/arm/data-rel-3.c (working copy) @@ -0,0 +1,11 @@ +/* { dg-options "-fPIC -mpic-data-is-text-relative" } */ +/* { dg-final { scan-assembler "j-\\(.LPIC" } } */ +/* { dg-final { scan-assembler-not "_GLOBAL_OFFSET_TABLE_-\\(.LPIC" } } */ +/* { dg-final { scan-assembler-not "j\\(GOT\\)" } } */ + +static int j; + +int *Foo () +{ + return &j; +}