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

["Joey Ye" <joey.ye@arm.com>]

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