[ARM] no-data-is-text-relative & msingle-pic-base

[ARM] no-data-is-text-relative & msingle-pic-base

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]

Ramana,
could you review this?

original thread https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00630.html, 
reproduced below:


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 though.

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 explicitly provided -m[no-]SPB.
2) clarified  the -m[no-]PDITR documentation.
3) Added some testcases -- there didn't appear to be any.

ok?

nathan

[-- Attachment #2: arm-pic.patch --]
[-- Type: text/x-patch, Size: 4319 bytes --]

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;
+}

Re: [ARM] no-data-is-text-relative & msingle-pic-base

[Ramana Radhakrishnan]

On 12/07/16 13:02, Nathan Sidwell wrote:
> Ramana,
> could you review this?


Sorry missed this.
> 
> original thread https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00630.html, reproduced below:
> 
> 
> 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 though.
> 
> 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 explicitly provided -m[no-]SPB.
> 2) clarified  the -m[no-]PDITR documentation.
> 3) Added some testcases -- there didn't appear to be any.
> 
> ok?
> 

Ok and thank you for the testcases. -mno-PDITR => -mSPB by default (in the absence of -mno-SPB on the command line) seems correct after having done the necessary archeology.

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.


Thanks,
Ramana

> nathan

Re: [ARM] no-data-is-text-relative & msingle-pic-base

[Nathan Sidwell]

On 07/12/16 12:01, Ramana Radhakrishnan wrote:

> 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 was my original thought too, but Joey told me that such use cases exist.

nathan

RE: [ARM] no-data-is-text-relative & msingle-pic-base

[Joey Ye]

Ramana, Nathan,

My opinion was "there is nothing wrong to run application -mno-pic-data-is-text-relative without -msingle-pic-base in a system that offset of data and text it fixed. I am not convenience we should ban it." 

However, what Ramana is suggesting is to error out -mno-PDITR with explicit -mno-SPB, which I don't have a strong preference embrace it or not as it will be just a rare and wired command line combination.

Thanks,
Joey

> -----Original Message-----
> From: Nathan Sidwell [mailto:nathanmsidwell@gmail.com] On Behalf Of
> Nathan Sidwell
> Sent: 12 July 2016 17:07
> To: Ramana Radhakrishnan
> Cc: GCC Patches; Joey Ye; nd
> Subject: Re: [ARM] no-data-is-text-relative & msingle-pic-base
> 
> On 07/12/16 12:01, Ramana Radhakrishnan wrote:
> 
> > 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 was my original thought too, but Joey told me that such use cases
> exist.
> 
> nathan