[patch] [arm] New option for PIC offset unfixed

[patch] [arm] New option for PIC offset unfixed

[Joey Ye]

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

For RTOS who need to relocate executable, PC relative and GOTOFF cannot be
used as the offset between any sections won't be fixed. Only GOT can be
used, just as VxWorks RTP does.

This patch introduces a new option enable user to choose between fixed
offset or not. Enabled for VxWorks RTP to keep its behavior unchanged.

Tested with arm-none-eabi make and VxWorks RTP small case

OK to trunk?

ChangeLog:
2013-11-12  Joey Ye  <joey.ye@arm.com>

	* config/arm/arm.c (arm_option_override):  Error if
	-mpic-offset-unfixed without -fpic, and set for
	VxWorks RTP.
	(legitimize_pic_address): Use arm_pic_offset_unfixed.
	(arm_assemble_integer): Likewise.
	* config/arm/arm.h
	(TARGET_DEFAULT_PIC_OFFSET_UNFIXED): New macro.
	* config/arm/arm.opt (mpic-offset-unfixed): New option.
	* doc/invoke.texi (-mpic-offset-unfixed): Doc for new option.

[-- Attachment #2: pic_offset_fixed-1112.patch --]
[-- Type: application/octet-stream, Size: 2904 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 7757e86..81cd5e7 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2504,6 +2504,12 @@ arm_option_override (void)
 	arm_pic_register = pic_register;
     }
 
+  if (TARGET_VXWORKS_RTP)
+    arm_pic_offset_unfixed = 1;
+
+  if (arm_pic_offset_unfixed != TARGET_DEFAULT_PIC_OFFSET_UNFIXED && !flag_pic)
+    error ("-mpic-offset-unfixed must be used with -fpic");
+
   /* Enable -mfix-cortex-m3-ldrd by default for Cortex-M3 cores.  */
   if (fix_cm3_ldrd == 2)
     {
@@ -6020,7 +6026,7 @@ legitimize_pic_address (rtx orig, enum machine_mode mode, rtx reg)
 	   || (GET_CODE (orig) == SYMBOL_REF &&
 	       SYMBOL_REF_LOCAL_P (orig)))
 	  && NEED_GOT_RELOC
-	  && !TARGET_VXWORKS_RTP)
+	  && !arm_pic_offset_unfixed)
 	insn = arm_pic_static_addr (orig, reg);
       else
 	{
@@ -21498,7 +21504,7 @@ arm_assemble_integer (rtx x, unsigned int size, int aligned_p)
 	{
 	  /* See legitimize_pic_address for an explanation of the
 	     TARGET_VXWORKS_RTP check.  */
-	  if (TARGET_VXWORKS_RTP
+	  if (arm_pic_offset_unfixed
 	      || (GET_CODE (x) == SYMBOL_REF && !SYMBOL_REF_LOCAL_P (x)))
 	    fputs ("(GOT)", asm_out_file);
 	  else
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 1781b75..17a672c 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -568,6 +568,10 @@ extern int prefer_neon_for_64bits;
 #define NEED_PLT_RELOC	0
 #endif
 
+#ifndef TARGET_DEFAULT_PIC_OFFSET_UNFIXED
+#define TARGET_DEFAULT_PIC_OFFSET_UNFIXED 0
+#endif
+
 /* Nonzero if we need to refer to the GOT with a PC-relative
    offset.  In other words, generate
 
diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
index 9b74038..acbe145 100644
--- a/gcc/config/arm/arm.opt
+++ b/gcc/config/arm/arm.opt
@@ -158,6 +158,10 @@ mlong-calls
 Target Report Mask(LONG_CALLS)
 Generate call insns as indirect calls, if necessary
 
+mpic-offset-unfixed
+Target Report Var(arm_pic_offset_unfixed) Init(TARGET_DEFAULT_PIC_OFFSET_UNFIXED)
+Assume inter-section offset is not the same between link time and run time.
+
 mpic-register=
 Target RejectNegative Joined Var(arm_pic_register_string)
 Specify the register to be used for PIC addressing
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 863e518..221db10 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12120,6 +12120,12 @@ before execution begins.
 Specify the register to be used for PIC addressing.  The default is R10
 unless stack-checking is enabled, when R9 is used.
 
+@item -mpic-offset-unfixed
+@opindex mpic-offset-unfixed
+Assume that each sections can be relocated separately at load time.
+Therefore, prevent PC relative and GOTOFF style relocations.  This is on
+by default for the VxWorks RTP target.
+
 @item -mpoke-function-name
 @opindex mpoke-function-name
 Write the name of each function into the text section, directly

Re: [patch] [arm] New option for PIC offset unfixed

[Richard Earnshaw]

On 12/11/13 06:59, Joey Ye wrote:
> For RTOS who need to relocate executable, PC relative and GOTOFF cannot be
> used as the offset between any sections won't be fixed. Only GOT can be
> used, just as VxWorks RTP does.
> 
> This patch introduces a new option enable user to choose between fixed
> offset or not. Enabled for VxWorks RTP to keep its behavior unchanged.
> 
> Tested with arm-none-eabi make and VxWorks RTP small case
> 
> OK to trunk?
> 
> ChangeLog:
> 2013-11-12  Joey Ye  <joey.ye@arm.com>
> 
> 	* config/arm/arm.c (arm_option_override):  Error if
> 	-mpic-offset-unfixed without -fpic, and set for
> 	VxWorks RTP.
> 	(legitimize_pic_address): Use arm_pic_offset_unfixed.
> 	(arm_assemble_integer): Likewise.
> 	* config/arm/arm.h
> 	(TARGET_DEFAULT_PIC_OFFSET_UNFIXED): New macro.
> 	* config/arm/arm.opt (mpic-offset-unfixed): New option.
> 	* doc/invoke.texi (-mpic-offset-unfixed): Doc for new option.
> 
> 

The name of the option and the documentation highlights that the
option's concept is confusing.

I think what you really need to do is to reverse the sense of the option
name and have

-mpic-data-is-text-relative

with the inverse (-mno-pic-data-is-text-relative) being the active value
that triggers for vxworks.  That is, PIC data being text-relative is the
default for all targets except vxworks.

R.

Oh, and at run time, we should be talking about segments, not sections!

RE: [patch] [arm] New option for PIC offset unfixed

[Joey Ye]

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

> -----Original Message-----
> From: Richard Earnshaw
> Sent: Tuesday, November 12, 2013 18:49
> To: Joey Ye
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [patch] [arm] New option for PIC offset unfixed
> 
> The name of the option and the documentation highlights that the
> option's concept is confusing.
> 
> I think what you really need to do is to reverse the sense of the option
> name and have
> 
> -mpic-data-is-text-relative
> 
> with the inverse (-mno-pic-data-is-text-relative) being the active value
> that triggers for vxworks.  That is, PIC data being text-relative is the
> default for all targets except vxworks.
> 
> R.
> 
> Oh, and at run time, we should be talking about segments, not sections!
Richard, Thanks for quick response.

Updated patch renamed the option, variables and macro. Also use segments in
document. OK?

2013-11-13  Joey Ye  <joey.ye@arm.com>

	* config/arm/arm.c (arm_option_override):  Error if
	-mpic-data-is-text-relative without -fpic, and set for
	VxWorks RTP.
	(legitimize_pic_address): Use arm_pic_data_is_text_relative
	(arm_assemble_integer): Likewise.
	* config/arm/arm.h
	(TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE): New macro.
	* config/arm/arm.opt (mpic-data-is-text-relative): New option.
	* doc/invoke.texi (-mpic-data-is-text-relative): Doc for new option.

[-- Attachment #2: pic_data_text_rel-1113.patch.txt --]
[-- Type: text/plain, Size: 3092 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 7757e86..fdd5684 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2504,6 +2504,13 @@ arm_option_override (void)
 	arm_pic_register = pic_register;
     }
 
+  if (TARGET_VXWORKS_RTP)
+    arm_pic_data_is_text_relative = 0;
+
+  if (arm_pic_data_is_text_relative != TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE
+      && !flag_pic)
+    error ("-mpic-data-is-text-relative must be used with -fpic");
+
   /* Enable -mfix-cortex-m3-ldrd by default for Cortex-M3 cores.  */
   if (fix_cm3_ldrd == 2)
     {
@@ -6020,7 +6027,7 @@ legitimize_pic_address (rtx orig, enum machine_mode mode, rtx reg)
 	   || (GET_CODE (orig) == SYMBOL_REF &&
 	       SYMBOL_REF_LOCAL_P (orig)))
 	  && NEED_GOT_RELOC
-	  && !TARGET_VXWORKS_RTP)
+	  && arm_pic_data_is_text_relative)
 	insn = arm_pic_static_addr (orig, reg);
       else
 	{
@@ -21498,7 +21505,7 @@ arm_assemble_integer (rtx x, unsigned int size, int aligned_p)
 	{
 	  /* See legitimize_pic_address for an explanation of the
 	     TARGET_VXWORKS_RTP check.  */
-	  if (TARGET_VXWORKS_RTP
+	  if (!arm_pic_data_is_text_relative
 	      || (GET_CODE (x) == SYMBOL_REF && !SYMBOL_REF_LOCAL_P (x)))
 	    fputs ("(GOT)", asm_out_file);
 	  else
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 1781b75..dbd841e 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -568,6 +568,10 @@ extern int prefer_neon_for_64bits;
 #define NEED_PLT_RELOC	0
 #endif
 
+#ifndef TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE
+#define TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE 1
+#endif
+
 /* Nonzero if we need to refer to the GOT with a PC-relative
    offset.  In other words, generate
 
diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
index 9b74038..fa0839a 100644
--- a/gcc/config/arm/arm.opt
+++ b/gcc/config/arm/arm.opt
@@ -158,6 +158,10 @@ mlong-calls
 Target Report Mask(LONG_CALLS)
 Generate call insns as indirect calls, if necessary
 
+mpic-data-is-text-relative
+Target Report Var(arm_pic_data_is_text_relative) Init(TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE)
+Assume data segments are relative to text segment.
+
 mpic-register=
 Target RejectNegative Joined Var(arm_pic_register_string)
 Specify the register to be used for PIC addressing
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 863e518..298fcc3 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12120,6 +12120,12 @@ before execution begins.
 Specify the register to be used for PIC addressing.  The default is R10
 unless stack-checking is enabled, when R9 is used.
 
+@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, prevent PC relative and GOTOFF style relocations to reference
+data.  This is on by default for targets other than VxWorks RTP.
+
 @item -mpoke-function-name
 @opindex mpoke-function-name
 Write the name of each function into the text section, directly

Re: [patch] [arm] New option for PIC offset unfixed

[Richard Earnshaw]

On 13/11/13 06:18, Joey Ye wrote:
>> -----Original Message-----
>> From: Richard Earnshaw
>> Sent: Tuesday, November 12, 2013 18:49
>> To: Joey Ye
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: Re: [patch] [arm] New option for PIC offset unfixed
>>
>> The name of the option and the documentation highlights that the
>> option's concept is confusing.
>>
>> I think what you really need to do is to reverse the sense of the option
>> name and have
>>
>> -mpic-data-is-text-relative
>>
>> with the inverse (-mno-pic-data-is-text-relative) being the active value
>> that triggers for vxworks.  That is, PIC data being text-relative is the
>> default for all targets except vxworks.
>>
>> R.
>>
>> Oh, and at run time, we should be talking about segments, not sections!
> Richard, Thanks for quick response.
> 
> Updated patch renamed the option, variables and macro. Also use segments in
> document. OK?
> 
> 2013-11-13  Joey Ye  <joey.ye@arm.com>
> 
> 	* config/arm/arm.c (arm_option_override):  Error if
> 	-mpic-data-is-text-relative without -fpic, and set for
> 	VxWorks RTP.
> 	(legitimize_pic_address): Use arm_pic_data_is_text_relative
> 	(arm_assemble_integer): Likewise.
> 	* config/arm/arm.h
> 	(TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE): New macro.
> 	* config/arm/arm.opt (mpic-data-is-text-relative): New option.
> 	* doc/invoke.texi (-mpic-data-is-text-relative): Doc for new option.
> 
> 
> pic_data_text_rel-1113.patch.txt
> 
> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 7757e86..fdd5684 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -2504,6 +2504,13 @@ arm_option_override (void)
>  	arm_pic_register = pic_register;
>      }
>  
> +  if (TARGET_VXWORKS_RTP)
> +    arm_pic_data_is_text_relative = 0;

Why is this needed?  Surely, even a VxWorks user should have the right
to force the compiler to behave differently.  You've set things up
through the default, now just accept what the user has asked for.


> +
> +  if (arm_pic_data_is_text_relative != TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE
> +      && !flag_pic)
> +    error ("-mpic-data-is-text-relative must be used with -fpic");

I'm not sure about this either.  The option should just be ignored when
not PIC.

> +
>    /* Enable -mfix-cortex-m3-ldrd by default for Cortex-M3 cores.  */
>    if (fix_cm3_ldrd == 2)
>      {
> @@ -6020,7 +6027,7 @@ legitimize_pic_address (rtx orig, enum machine_mode mode, rtx reg)
>  	   || (GET_CODE (orig) == SYMBOL_REF &&
>  	       SYMBOL_REF_LOCAL_P (orig)))
>  	  && NEED_GOT_RELOC
> -	  && !TARGET_VXWORKS_RTP)
> +	  && arm_pic_data_is_text_relative)
>  	insn = arm_pic_static_addr (orig, reg);
>        else
>  	{
> @@ -21498,7 +21505,7 @@ arm_assemble_integer (rtx x, unsigned int size, int aligned_p)
>  	{
>  	  /* See legitimize_pic_address for an explanation of the
>  	     TARGET_VXWORKS_RTP check.  */
> -	  if (TARGET_VXWORKS_RTP
> +	  if (!arm_pic_data_is_text_relative
>  	      || (GET_CODE (x) == SYMBOL_REF && !SYMBOL_REF_LOCAL_P (x)))
>  	    fputs ("(GOT)", asm_out_file);
>  	  else
> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> index 1781b75..dbd841e 100644
> --- a/gcc/config/arm/arm.h
> +++ b/gcc/config/arm/arm.h
> @@ -568,6 +568,10 @@ extern int prefer_neon_for_64bits;
>  #define NEED_PLT_RELOC	0
>  #endif
>  
> +#ifndef TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE
> +#define TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE 1
> +#endif
> +
>  /* Nonzero if we need to refer to the GOT with a PC-relative
>     offset.  In other words, generate
>  
> diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
> index 9b74038..fa0839a 100644
> --- a/gcc/config/arm/arm.opt
> +++ b/gcc/config/arm/arm.opt
> @@ -158,6 +158,10 @@ mlong-calls
>  Target Report Mask(LONG_CALLS)
>  Generate call insns as indirect calls, if necessary
>  
> +mpic-data-is-text-relative
> +Target Report Var(arm_pic_data_is_text_relative) Init(TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE)
> +Assume data segments are relative to text segment.
                           ^
Assume data segment will be loaded at a fixed relative offset to the
text segment.

> +
>  mpic-register=
>  Target RejectNegative Joined Var(arm_pic_register_string)
>  Specify the register to be used for PIC addressing
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 863e518..298fcc3 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -12120,6 +12120,12 @@ before execution begins.
>  Specify the register to be used for PIC addressing.  The default is R10
>  unless stack-checking is enabled, when R9 is used.
>  
> +@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, prevent PC relative and GOTOFF style relocations to reference
> +data.  

I think the sense of this sentence is now backwards.  I'd also try to
avoid GOTOFF in the user part of the manual.

> This is on by default for targets other than VxWorks RTP.
> +
>  @item -mpoke-function-name
>  @opindex mpoke-function-name
>  Write the name of each function into the text section, directly
> 

R.

RE: [patch] [arm] New option for PIC offset unfixed

[Joey Ye]

> -----Original Message-----
> From: Richard Earnshaw
> Sent: Wednesday, November 13, 2013 17:49
> To: Joey Ye
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [patch] [arm] New option for PIC offset unfixed
> 
> On 13/11/13 06:18, Joey Ye wrote:
> >> -----Original Message-----
> >> From: Richard Earnshaw
> >> Sent: Tuesday, November 12, 2013 18:49
> >> To: Joey Ye
> >> Cc: gcc-patches@gcc.gnu.org
> >> Subject: Re: [patch] [arm] New option for PIC offset unfixed
> >>
> >> The name of the option and the documentation highlights that the
> >> option's concept is confusing.
> >>
> >> I think what you really need to do is to reverse the sense of the
> >> option name and have
> >>
> >> -mpic-data-is-text-relative
> >>
> >> with the inverse (-mno-pic-data-is-text-relative) being the active
> >> value that triggers for vxworks.  That is, PIC data being
> >> text-relative is the default for all targets except vxworks.
> >>
> >> R.
> >>
> >> Oh, and at run time, we should be talking about segments, not sections!
> > Richard, Thanks for quick response.
> >
> > Updated patch renamed the option, variables and macro. Also use
> > segments in document. OK?
> >
> > 2013-11-13  Joey Ye  <joey.ye@arm.com>
> >
> > 	* config/arm/arm.c (arm_option_override):  Error if
> > 	-mpic-data-is-text-relative without -fpic, and set for
> > 	VxWorks RTP.
> > 	(legitimize_pic_address): Use arm_pic_data_is_text_relative
> > 	(arm_assemble_integer): Likewise.
> > 	* config/arm/arm.h
> > 	(TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE): New macro.
> > 	* config/arm/arm.opt (mpic-data-is-text-relative): New option.
> > 	* doc/invoke.texi (-mpic-data-is-text-relative): Doc for new option.
> >
> >
> > pic_data_text_rel-1113.patch.txt
> >
> >
> > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index
> > 7757e86..fdd5684 100644
> > --- a/gcc/config/arm/arm.c
> > +++ b/gcc/config/arm/arm.c
> > @@ -2504,6 +2504,13 @@ arm_option_override (void)
> >  	arm_pic_register = pic_register;
> >      }
> >
> > +  if (TARGET_VXWORKS_RTP)
> > +    arm_pic_data_is_text_relative = 0;
> 
> Why is this needed?  Surely, even a VxWorks user should have the right to
> force the compiler to behave differently.  You've set things up through
the
> default, now just accept what the user has asked for.
The reason is that TARGET_VXWORKS_RTP isn't a compile time value to initiate
arm.opt. Instead it is true only when -mrtp is specified in runtime. Also
enable text relative may result in runtime error on vxworks, it is better to
prevent it here.
> 
> 
> > +
> > +  if (arm_pic_data_is_text_relative !=
> TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE
> > +      && !flag_pic)
> > +    error ("-mpic-data-is-text-relative must be used with -fpic");
> 
> I'm not sure about this either.  The option should just be ignored when
not
> PIC.
It is ignored if -fpic isn't given. I'm OK to remove the error here, while
Ramana preferred to error it.

> 
> > +
> >    /* Enable -mfix-cortex-m3-ldrd by default for Cortex-M3 cores.  */
> >    if (fix_cm3_ldrd == 2)
> >      {
> > @@ -6020,7 +6027,7 @@ legitimize_pic_address (rtx orig, enum
> machine_mode mode, rtx reg)
> >  	   || (GET_CODE (orig) == SYMBOL_REF &&
> >  	       SYMBOL_REF_LOCAL_P (orig)))
> >  	  && NEED_GOT_RELOC
> > -	  && !TARGET_VXWORKS_RTP)
> > +	  && arm_pic_data_is_text_relative)
> >  	insn = arm_pic_static_addr (orig, reg);
> >        else
> >  	{
> > @@ -21498,7 +21505,7 @@ arm_assemble_integer (rtx x, unsigned int size,
> int aligned_p)
> >  	{
> >  	  /* See legitimize_pic_address for an explanation of the
> >  	     TARGET_VXWORKS_RTP check.  */
> > -	  if (TARGET_VXWORKS_RTP
> > +	  if (!arm_pic_data_is_text_relative
> >  	      || (GET_CODE (x) == SYMBOL_REF && !SYMBOL_REF_LOCAL_P
> (x)))
> >  	    fputs ("(GOT)", asm_out_file);
> >  	  else
> > diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index
> > 1781b75..dbd841e 100644
> > --- a/gcc/config/arm/arm.h
> > +++ b/gcc/config/arm/arm.h
> > @@ -568,6 +568,10 @@ extern int prefer_neon_for_64bits;
> >  #define NEED_PLT_RELOC	0
> >  #endif
> >
> > +#ifndef TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE
> > +#define TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE 1 #endif
> > +
> >  /* Nonzero if we need to refer to the GOT with a PC-relative
> >     offset.  In other words, generate
> >
> > diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt index
> > 9b74038..fa0839a 100644
> > --- a/gcc/config/arm/arm.opt
> > +++ b/gcc/config/arm/arm.opt
> > @@ -158,6 +158,10 @@ mlong-calls
> >  Target Report Mask(LONG_CALLS)
> >  Generate call insns as indirect calls, if necessary
> >
> > +mpic-data-is-text-relative
> > +Target Report Var(arm_pic_data_is_text_relative)
> > +Init(TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE)
> > +Assume data segments are relative to text segment.
>                            ^
> Assume data segment will be loaded at a fixed relative offset to the text
> segment.
Accepted
> 
> > +
> >  mpic-register=
> >  Target RejectNegative Joined Var(arm_pic_register_string)  Specify
> > the register to be used for PIC addressing diff --git
> > a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 863e518..298fcc3
> > 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -12120,6 +12120,12 @@ before execution begins.
> >  Specify the register to be used for PIC addressing.  The default is
> > R10  unless stack-checking is enabled, when R9 is used.
> >
> > +@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, prevent PC relative and GOTOFF style relocations to
> > +reference data.
> 
> I think the sense of this sentence is now backwards.  I'd also try to
avoid
> GOTOFF in the user part of the manual.
How about
"Therefore, prevent addressing data with relocation types that doesn't apply
in such circumstance."

> 
> > This is on by default for targets other than VxWorks RTP.
> > +
> >  @item -mpoke-function-name
> >  @opindex mpoke-function-name
> >  Write the name of each function into the text section, directly
> >
> 
> R.

Re: [patch] [arm] New option for PIC offset unfixed

[Richard Earnshaw]

On 13/11/13 10:20, Joey Ye wrote:
>>> +@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, prevent PC relative and GOTOFF style relocations to
>>> > > +reference data.
>> > 
>> > I think the sense of this sentence is now backwards.  I'd also try to
> avoid
>> > GOTOFF in the user part of the manual.
> How about
> "Therefore, prevent addressing data with relocation types that doesn't apply
> in such circumstance."
> 
>> > 

No, that's still backwards.  Remember, the option is now
pic-data-is-text-relative, so the option /permits/ addressing data using
PC-relative operations.

R.

RE: [patch] [arm] New option for PIC offset unfixed

[Joey Ye]

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

This patch address all comments.

Thanks,
Joey

> -----Original Message-----
> From: Richard Earnshaw
> Sent: Wednesday, November 13, 2013 19:07
> To: Joey Ye
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [patch] [arm] New option for PIC offset unfixed
> 
> On 13/11/13 10:20, Joey Ye wrote:
> >>> +@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, prevent PC relative and GOTOFF style relocations to
> >>> > > +reference data.
> >> >
> >> > I think the sense of this sentence is now backwards.  I'd also try
> >> > to
> > avoid
> >> > GOTOFF in the user part of the manual.
> > How about
> > "Therefore, prevent addressing data with relocation types that doesn't
> > apply in such circumstance."
> >
> >> >
> 
> No, that's still backwards.  Remember, the option is now pic-data-is-text-
> relative, so the option /permits/ addressing data using PC-relative
operations.
> 
> R.

[-- Attachment #2: pic_data_text_rel-1114.patch.txt --]
[-- Type: text/plain, Size: 2498 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 7757e86..5a95399 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2504,6 +2504,11 @@ arm_option_override (void)
 	arm_pic_register = pic_register;
     }
 
+  if (arm_pic_data_is_text_relative < 0 && TARGET_VXWORKS_RTP)
+    arm_pic_data_is_text_relative = 0;
+  else
+    arm_pic_data_is_text_relative = 1;
+
   /* Enable -mfix-cortex-m3-ldrd by default for Cortex-M3 cores.  */
   if (fix_cm3_ldrd == 2)
     {
@@ -6020,7 +6025,7 @@ legitimize_pic_address (rtx orig, enum machine_mode mode, rtx reg)
 	   || (GET_CODE (orig) == SYMBOL_REF &&
 	       SYMBOL_REF_LOCAL_P (orig)))
 	  && NEED_GOT_RELOC
-	  && !TARGET_VXWORKS_RTP)
+	  && arm_pic_data_is_text_relative)
 	insn = arm_pic_static_addr (orig, reg);
       else
 	{
@@ -21498,7 +21503,7 @@ arm_assemble_integer (rtx x, unsigned int size, int aligned_p)
 	{
 	  /* See legitimize_pic_address for an explanation of the
 	     TARGET_VXWORKS_RTP check.  */
-	  if (TARGET_VXWORKS_RTP
+	  if (!arm_pic_data_is_text_relative
 	      || (GET_CODE (x) == SYMBOL_REF && !SYMBOL_REF_LOCAL_P (x)))
 	    fputs ("(GOT)", asm_out_file);
 	  else
diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
index 9b74038..adac749 100644
--- a/gcc/config/arm/arm.opt
+++ b/gcc/config/arm/arm.opt
@@ -158,6 +158,10 @@ mlong-calls
 Target Report Mask(LONG_CALLS)
 Generate call insns as indirect calls, if necessary
 
+mpic-data-is-text-relative
+Target Report Var(arm_pic_data_is_text_relative) Init(-1)
+Assume data segments are relative to text segment.
+
 mpic-register=
 Target RejectNegative Joined Var(arm_pic_register_string)
 Specify the register to be used for PIC addressing
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 863e518..fbe77e6 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12120,6 +12120,12 @@ before execution begins.
 Specify the register to be used for PIC addressing.  The default is R10
 unless stack-checking is enabled, when R9 is used.
 
+@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.
+
 @item -mpoke-function-name
 @opindex mpoke-function-name
 Write the name of each function into the text section, directly

Re: [patch] [arm] New option for PIC offset unfixed

[Richard Earnshaw]

On 13/11/13 10:20, Joey Ye wrote:
>>> +  if (TARGET_VXWORKS_RTP)
>>> > > +    arm_pic_data_is_text_relative = 0;
>> > 
>> > Why is this needed?  Surely, even a VxWorks user should have the right to
>> > force the compiler to behave differently.  You've set things up through
> the
>> > default, now just accept what the user has asked for.
> The reason is that TARGET_VXWORKS_RTP isn't a compile time value to initiate
> arm.opt. Instead it is true only when -mrtp is specified in runtime. Also
> enable text relative may result in runtime error on vxworks, it is better to
> prevent it here.

I'd be happier if this was only done if the command-line option was not
explicitly set on the command line.

R.

Re: [patch] [arm] New option for PIC offset unfixed

[Richard Earnshaw]

On 13/11/13 15:57, Joey Ye wrote:
> 
> 
>> -----Original Message-----
>> From: Richard Earnshaw
>> Sent: Wednesday, November 13, 2013 19:17
>> To: Joey Ye
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: Re: [patch] [arm] New option for PIC offset unfixed
>>
>> On 13/11/13 10:20, Joey Ye wrote:
>>>>> +  if (TARGET_VXWORKS_RTP)
>>>>>>> +    arm_pic_data_is_text_relative = 0;
>>>>>
>>>>> Why is this needed?  Surely, even a VxWorks user should have the
>>>>> right to force the compiler to behave differently.  You've set
>>>>> things up through
>>> the
>>>>> default, now just accept what the user has asked for.
>>> The reason is that TARGET_VXWORKS_RTP isn't a compile time value to
>>> initiate arm.opt. Instead it is true only when -mrtp is specified in
>>> runtime. Also enable text relative may result in runtime error on
>>> vxworks, it is better to prevent it here.
>>
>> I'd be happier if this was only done if the command-line option was not
>> explicitly set on the command line.
> So you are suggesting change like this:
> + Target Report Var(arm_pic_data_is_text_relative) Init(-1)
> 
> +   if (arm_pic_data_is_text_relative < 0 && TARGET_VXWORKS_RTP)
> +     arm_pic_data_is_text_relative = 0;
> +   else
> +     arm_pic_data_is_text_relative = 1;
> 

No, use the global_options_set structure to find out if the user has set
the value.

RE: [patch] [arm] New option for PIC offset unfixed

[Joey Ye]

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

> -----Original Message-----
> From: Richard Earnshaw
> Sent: Thursday, November 14, 2013 0:57
> To: Joey Ye
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [patch] [arm] New option for PIC offset unfixed
> 
> > So you are suggesting change like this:
> > + Target Report Var(arm_pic_data_is_text_relative) Init(-1)
> >
> > +   if (arm_pic_data_is_text_relative < 0 && TARGET_VXWORKS_RTP)
> > +     arm_pic_data_is_text_relative = 0;
> > +   else
> > +     arm_pic_data_is_text_relative = 1;
> >
> 
> No, use the global_options_set structure to find out if the user has set
the
> value.
Thank pointing this out. Here is the latest patch with global_options_set

[-- Attachment #2: pic_data_text_rel-1114.patch.txt --]
[-- Type: text/plain, Size: 2974 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 7757e86..3af8293 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2504,6 +2504,10 @@ 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)
     {
@@ -6020,7 +6024,7 @@ legitimize_pic_address (rtx orig, enum machine_mode mode, rtx reg)
 	   || (GET_CODE (orig) == SYMBOL_REF &&
 	       SYMBOL_REF_LOCAL_P (orig)))
 	  && NEED_GOT_RELOC
-	  && !TARGET_VXWORKS_RTP)
+	  && arm_pic_data_is_text_relative)
 	insn = arm_pic_static_addr (orig, reg);
       else
 	{
@@ -21498,7 +21502,7 @@ arm_assemble_integer (rtx x, unsigned int size, int aligned_p)
 	{
 	  /* See legitimize_pic_address for an explanation of the
 	     TARGET_VXWORKS_RTP check.  */
-	  if (TARGET_VXWORKS_RTP
+	  if (!arm_pic_data_is_text_relative
 	      || (GET_CODE (x) == SYMBOL_REF && !SYMBOL_REF_LOCAL_P (x)))
 	    fputs ("(GOT)", asm_out_file);
 	  else
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 1781b75..dbd841e 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -568,6 +568,10 @@ extern int prefer_neon_for_64bits;
 #define NEED_PLT_RELOC	0
 #endif
 
+#ifndef TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE
+#define TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE 1
+#endif
+
 /* Nonzero if we need to refer to the GOT with a PC-relative
    offset.  In other words, generate
 
diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
index 9b74038..fa0839a 100644
--- a/gcc/config/arm/arm.opt
+++ b/gcc/config/arm/arm.opt
@@ -158,6 +158,10 @@ mlong-calls
 Target Report Mask(LONG_CALLS)
 Generate call insns as indirect calls, if necessary
 
+mpic-data-is-text-relative
+Target Report Var(arm_pic_data_is_text_relative) Init(TARGET_DEFAULT_PIC_DATA_IS_TEXT_RELATIVE)
+Assume data segments are relative to text segment.
+
 mpic-register=
 Target RejectNegative Joined Var(arm_pic_register_string)
 Specify the register to be used for PIC addressing
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 863e518..fbe77e6 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12120,6 +12120,12 @@ before execution begins.
 Specify the register to be used for PIC addressing.  The default is R10
 unless stack-checking is enabled, when R9 is used.
 
+@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.
+
 @item -mpoke-function-name
 @opindex mpoke-function-name
 Write the name of each function into the text section, directly

Re: [patch] [arm] New option for PIC offset unfixed

[Richard Earnshaw]

On 14/11/13 08:23, Joey Ye wrote:
>> -----Original Message-----
>> From: Richard Earnshaw
>> Sent: Thursday, November 14, 2013 0:57
>> To: Joey Ye
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: Re: [patch] [arm] New option for PIC offset unfixed
>>
>>> So you are suggesting change like this:
>>> + Target Report Var(arm_pic_data_is_text_relative) Init(-1)
>>>
>>> +   if (arm_pic_data_is_text_relative < 0 && TARGET_VXWORKS_RTP)
>>> +     arm_pic_data_is_text_relative = 0;
>>> +   else
>>> +     arm_pic_data_is_text_relative = 1;
>>>
>>
>> No, use the global_options_set structure to find out if the user has set
> the
>> value.
> Thank pointing this out. Here is the latest patch with global_options_set
> 
> 

This is OK.

However, don't you also need to fix the other references to
TARGET_VXWORKS_RTP; eg pic_offset_arm in arm.md?

R.

RE: [patch] [arm] New option for PIC offset unfixed

[Joey Ye]

> -----Original Message-----
> From: Richard Earnshaw
> Sent: Thursday, November 14, 2013 18:00
> To: Joey Ye
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [patch] [arm] New option for PIC offset unfixed
> 
> On 14/11/13 08:23, Joey Ye wrote:
> >> -----Original Message-----
> >> From: Richard Earnshaw
> >> Sent: Thursday, November 14, 2013 0:57
> >> To: Joey Ye
> >> Cc: gcc-patches@gcc.gnu.org
> >> Subject: Re: [patch] [arm] New option for PIC offset unfixed
> >>
> >>> So you are suggesting change like this:
> >>> + Target Report Var(arm_pic_data_is_text_relative) Init(-1)
> >>>
> >>> +   if (arm_pic_data_is_text_relative < 0 && TARGET_VXWORKS_RTP)
> >>> +     arm_pic_data_is_text_relative = 0;
> >>> +   else
> >>> +     arm_pic_data_is_text_relative = 1;
> >>>
> >>
> >> No, use the global_options_set structure to find out if the user has
> >> set
> > the
> >> value.
> > Thank pointing this out. Here is the latest patch with
> > global_options_set
> >
> >
> 
> This is OK.
Thanks!
> 
> However, don't you also need to fix the other references to
> TARGET_VXWORKS_RTP; eg pic_offset_arm in arm.md?
The only reference of TARGET_VXWORKS_RTP in arm.md is define_insn
pic_offset_arm, which is used in arm_load_pic_register as
  if (TARGET_VXWORKS_RTP)
    {
      ...
      emit_insn (gen_pic_offset_arm (pic_reg, pic_reg, pic_tmp));
    }

Apparently it is a VxWorks specific pattern, I don't think it should be
changed, though I'm not 100% sure.

- Joey