Re: [RFC] PowerPC select_section / unique_section

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

	To use the varasm decl analysis machinery in PowerPC, I suggest
defining the functions to accept a "pic" argument as appended, with
appropriate changes to target-specific callers of decl_readonly_section.
Then the functions are flexible-enough to be called from more targets.

	Comments?  Geoff?  Richard?

David


	* output.h (decl_readonly_section): Add pic parameter.
	* varasm.c (default_section_type_flags): Use flag_pic as pic
	argument to decl_readonly_section.
	(default_select_section): Same.
	(categorize_decl_for_section): Add pic parameter. Use it in place
	of flag_pic.
	(decl_readonly_section): Add pic paramter.
	(default_elf_select_section): Use flag_pic as pic argument to
	categorize_decl_for_section. 
	(default_unique_section): Same.
	

Index: output.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/output.h,v
retrieving revision 1.109
diff -c -p -r1.109 output.h
*** output.h	21 Aug 2002 02:41:44 -0000	1.109
--- output.h	22 Aug 2002 19:42:11 -0000
*************** extern rtx this_is_asm_operands;
*** 467,473 ****
  
  /* Decide whether DECL needs to be in a writable section.
     RELOC is the same as for SELECT_SECTION.  */
! extern bool decl_readonly_section PARAMS ((tree, int));
  
  /* User label prefix in effect for this compilation.  */
  extern const char *user_label_prefix;
--- 467,473 ----
  
  /* Decide whether DECL needs to be in a writable section.
     RELOC is the same as for SELECT_SECTION.  */
! extern bool decl_readonly_section PARAMS ((tree, int, int));
  
  /* User label prefix in effect for this compilation.  */
  extern const char *user_label_prefix;
Index: varasm.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/varasm.c,v
retrieving revision 1.303
diff -c -p -r1.303 varasm.c
*** varasm.c	21 Aug 2002 02:41:44 -0000	1.303
--- varasm.c	22 Aug 2002 19:42:11 -0000
*************** default_section_type_flags (decl, name, 
*** 4695,4701 ****
  
    if (decl && TREE_CODE (decl) == FUNCTION_DECL)
      flags = SECTION_CODE;
!   else if (decl && decl_readonly_section (decl, reloc))
      flags = 0;
    else
      flags = SECTION_WRITE;
--- 4695,4701 ----
  
    if (decl && TREE_CODE (decl) == FUNCTION_DECL)
      flags = SECTION_CODE;
!   else if (decl && decl_readonly_section (decl, reloc, flag_pic))
      flags = 0;
    else
      flags = SECTION_WRITE;
*************** default_select_section (decl, reloc, ali
*** 4855,4861 ****
  
    if (DECL_P (decl))
      {
!       if (decl_readonly_section (decl, reloc))
  	readonly = true;
      }
    else if (TREE_CODE (decl) == CONSTRUCTOR)
--- 4855,4861 ----
  
    if (DECL_P (decl))
      {
!       if (decl_readonly_section (decl, reloc, flag_pic))
  	readonly = true;
      }
    else if (TREE_CODE (decl) == CONSTRUCTOR)
*************** enum section_category
*** 4913,4924 ****
    SECCAT_TBSS
  };
  
! static enum section_category categorize_decl_for_section PARAMS ((tree, int));
  
  static enum section_category
! categorize_decl_for_section (decl, reloc)
       tree decl;
       int reloc;
  {
    enum section_category ret;
  
--- 4913,4926 ----
    SECCAT_TBSS
  };
  
! static enum section_category
! categorize_decl_for_section PARAMS ((tree, int, int));
  
  static enum section_category
! categorize_decl_for_section (decl, reloc, pic)
       tree decl;
       int reloc;
+      int pic;
  {
    enum section_category ret;
  
*************** categorize_decl_for_section (decl, reloc
*** 4940,4955 ****
  	       || TREE_SIDE_EFFECTS (decl)
  	       || ! TREE_CONSTANT (DECL_INITIAL (decl)))
  	{
! 	  if (flag_pic && (reloc & 2))
  	    ret = SECCAT_DATA_REL;
! 	  else if (flag_pic && reloc)
  	    ret = SECCAT_DATA_REL_LOCAL;
  	  else
  	    ret = SECCAT_DATA;
  	}
!       else if (flag_pic && (reloc & 2))
  	ret = SECCAT_DATA_REL_RO;
!       else if (flag_pic && reloc)
  	ret = SECCAT_DATA_REL_RO_LOCAL;
        else if (flag_merge_constants < 2)
  	/* C and C++ don't allow different variables to share the same
--- 4942,4957 ----
  	       || TREE_SIDE_EFFECTS (decl)
  	       || ! TREE_CONSTANT (DECL_INITIAL (decl)))
  	{
! 	  if (pic && (reloc & 2))
  	    ret = SECCAT_DATA_REL;
! 	  else if (pic && reloc)
  	    ret = SECCAT_DATA_REL_LOCAL;
  	  else
  	    ret = SECCAT_DATA;
  	}
!       else if (pic && (reloc & 2))
  	ret = SECCAT_DATA_REL_RO;
!       else if (pic && reloc)
  	ret = SECCAT_DATA_REL_RO_LOCAL;
        else if (flag_merge_constants < 2)
  	/* C and C++ don't allow different variables to share the same
*************** categorize_decl_for_section (decl, reloc
*** 4963,4969 ****
      }
    else if (TREE_CODE (decl) == CONSTRUCTOR)
      {
!       if ((flag_pic && reloc)
  	  || TREE_SIDE_EFFECTS (decl)
  	  || ! TREE_CONSTANT (decl))
  	ret = SECCAT_DATA;
--- 4965,4971 ----
      }
    else if (TREE_CODE (decl) == CONSTRUCTOR)
      {
!       if ((pic && reloc)
  	  || TREE_SIDE_EFFECTS (decl)
  	  || ! TREE_CONSTANT (decl))
  	ret = SECCAT_DATA;
*************** categorize_decl_for_section (decl, reloc
*** 4995,5005 ****
  }
  
  bool
! decl_readonly_section (decl, reloc)
       tree decl;
       int reloc;
  {
!   switch (categorize_decl_for_section (decl, reloc))
      {
      case SECCAT_RODATA:
      case SECCAT_RODATA_MERGE_STR:
--- 4997,5008 ----
  }
  
  bool
! decl_readonly_section (decl, reloc, pic)
       tree decl;
       int reloc;
+      int pic;
  {
!   switch (categorize_decl_for_section (decl, reloc, pic))
      {
      case SECCAT_RODATA:
      case SECCAT_RODATA_MERGE_STR:
*************** default_elf_select_section (decl, reloc,
*** 5021,5027 ****
       int reloc;
       unsigned HOST_WIDE_INT align;
  {
!   switch (categorize_decl_for_section (decl, reloc))
      {
      case SECCAT_TEXT:
        /* We're not supposed to be called on FUNCTION_DECLs.  */
--- 5024,5030 ----
       int reloc;
       unsigned HOST_WIDE_INT align;
  {
!   switch (categorize_decl_for_section (decl, reloc, flag_pic))
      {
      case SECCAT_TEXT:
        /* We're not supposed to be called on FUNCTION_DECLs.  */
*************** default_unique_section (decl, reloc)
*** 5090,5096 ****
    size_t nlen, plen;
    char *string;
  
!   switch (categorize_decl_for_section (decl, reloc))
      {
      case SECCAT_TEXT:
        prefix = one_only ? ".gnu.linkonce.t." : ".text.";
--- 5093,5099 ----
    size_t nlen, plen;
    char *string;
  
!   switch (categorize_decl_for_section (decl, reloc, flag_pic))
      {
      case SECCAT_TEXT:
        prefix = one_only ? ".gnu.linkonce.t." : ".text.";

Re: [RFC] PowerPC select_section / unique_section

[Richard Henderson]

On Thu, Aug 22, 2002 at 03:59:16PM -0400, David Edelsohn wrote:
> 	* output.h (decl_readonly_section): Add pic parameter.
> 	* varasm.c (default_section_type_flags): Use flag_pic as pic
> 	argument to decl_readonly_section.
> 	(default_select_section): Same.
> 	(categorize_decl_for_section): Add pic parameter. Use it in place
> 	of flag_pic.
> 	(decl_readonly_section): Add pic paramter.
> 	(default_elf_select_section): Use flag_pic as pic argument to
> 	categorize_decl_for_section. 
> 	(default_unique_section): Same.

For 3.2 you need to make this as minimal as possible.  As is
you'd need to touch arm, i386, mcore and mips.  Saving and
restoring flag_pic around the calls in the rs6000 backend is
really best.

For 3.4, I'd _really_ like to see the ppc backend cleaned up
to use targetm.in_small_data_p and the standard varasm.c
functions.  I guess I don't know what I'd want to happen with
flag_pic.  I don't really mind the backend save/restore thing;
possibly a better solution is to introduce a targetm member
that forces flag_pic to be considered true wrt relocs.
Yes, I'm aware that the existing varasm.c code can't handle
the ppc-eabi .sdata2 section; that can be fixed.

For 3.3... I have no idea.  I guess there's no point in writing
three patches, so you might as well apply the 3.2 patch there
as well.


r~

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Richard Henderson writes:

Richard> Yes, I'm aware that the existing varasm.c code can't handle
Richard> the ppc-eabi .sdata2 section; that can be fixed.

	How about the following patch for handling PPC EABI sdata2 in
varasm.c?  In my proposal targetm.in_small_data_p should have the logic
for allowing or disallowing small readonly data if it only is allowed in
some contexts (like eABI versus SVR4), as opposed to adding another
target-defined function which has some of the functionality of
in_small_data_p.

Thanks, David


	* varasm.c (section_category): Add SECCAT_SRODATA.
	(categorize_decl_for_section): Return SECCAT_SRODATA for sdata if
	READONLY_SDATA_SECTION defined.
	(decl_readonly_section_1): True for SECCAT_SRODATA also.
	(default_elf_select_section_1): Map SECCAT_SRODATA to .sdata2.
	(default_unique_section_1): Likewise.

Index: varasm.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/varasm.c,v
retrieving revision 1.304
diff -c -p -r1.304 varasm.c
*** varasm.c	4 Sep 2002 01:56:11 -0000	1.304
--- varasm.c	4 Sep 2002 04:09:27 -0000
*************** enum section_category
*** 4898,4903 ****
--- 4898,4904 ----
    SECCAT_RODATA_MERGE_STR,
    SECCAT_RODATA_MERGE_STR_INIT,
    SECCAT_RODATA_MERGE_CONST,
+   SECCAT_SRODATA,
  
    SECCAT_DATA,
  
*************** categorize_decl_for_section (decl, reloc
*** 4999,5004 ****
--- 5000,5009 ----
      {
        if (ret == SECCAT_BSS)
  	ret = SECCAT_SBSS;
+ #ifdef READONLY_SDATA_SECTION
+       else if (ret == SECCAT_RODATA)
+ 	ret = SECCAT_SRODATA;
+ #endif
        else
  	ret = SECCAT_SDATA;
      }
*************** decl_readonly_section_1 (decl, reloc, sh
*** 5026,5031 ****
--- 5031,5037 ----
      case SECCAT_RODATA_MERGE_STR:
      case SECCAT_RODATA_MERGE_STR_INIT:
      case SECCAT_RODATA_MERGE_CONST:
+     case SECCAT_SRODATA:
        return true;
        break;
      default:
*************** default_elf_select_section_1 (decl, relo
*** 5069,5074 ****
--- 5075,5083 ----
      case SECCAT_RODATA_MERGE_CONST:
        mergeable_constant_section (DECL_MODE (decl), align, 0);
        break;
+     case SECCAT_SRODATA:
+       named_section (NULL_TREE, ".sdata2", reloc);
+       break;
      case SECCAT_DATA:
        data_section ();
        break;
*************** default_unique_section_1 (decl, reloc, s
*** 5140,5145 ****
--- 5149,5157 ----
      case SECCAT_RODATA_MERGE_STR_INIT:
      case SECCAT_RODATA_MERGE_CONST:
        prefix = one_only ? ".gnu.linkonce.r." : ".rodata.";
+       break;
+     case SECCAT_SRODATA:
+       prefix = one_only ? ".gnu.linkonce.s2." : ".sdata2.";
        break;
      case SECCAT_DATA:
      case SECCAT_DATA_REL:

Re: [RFC] PowerPC select_section / unique_section

[Richard Henderson]

On Wed, Sep 04, 2002 at 12:15:27AM -0400, David Edelsohn wrote:
> 	How about the following patch for handling PPC EABI sdata2 in
> varasm.c?  In my proposal targetm.in_small_data_p should have the logic
> for allowing or disallowing small readonly data if it only is allowed in
> some contexts (like eABI versus SVR4), as opposed to adding another
> target-defined function which has some of the functionality of
> in_small_data_p.

I'm not fond of the ifdef.  A boolean on the target structure
would be better IMO.


r~

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Richard Henderson writes:

Richard> I'm not fond of the ifdef.  A boolean on the target structure
Richard> would be better IMO.

	Okay, but what concerns me is the overlap between
targetm.in_small_data_p and the new targetm.readonly_small_data_p.  The
logic will be split between those two, nested functions which will
interact in complicated and potentially confusing ways.

	Can we combine the logic into targetm.in_small_data_p by adding a
readonly boolean parameter to the in_small_data_p call, e.g.,

  else if ((*targetm.in_small_data_p) (decl, ret == SECCAT_RODATA))

and modifying the in_small_data_p functions in alpha and ia64 to return
false if the readonly argument is true?  The ppc code can check on which
ABI is in effect.  This way in_small_data_p controls whether the sdata
categorization code is executed for readonly objects removing the need for
the #ifdef.

Thanks, David

Re: [RFC] PowerPC select_section / unique_section

[Richard Henderson]

On Wed, Sep 04, 2002 at 01:09:58AM -0400, David Edelsohn wrote:
> Richard> I'm not fond of the ifdef.  A boolean on the target structure
> Richard> would be better IMO.
> 
> 	Okay, but what concerns me is the overlap between
> targetm.in_small_data_p and the new targetm.readonly_small_data_p.  The
> logic will be split between those two, nested functions which will
> interact in complicated and potentially confusing ways.

Err, no, that's not what I mean.  A boolean field (not function)
that says whether small data continues to be read only as well.



r~

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Richard Henderson writes:

Richard> Err, no, that's not what I mean.  A boolean field (not function)
Richard> that says whether small data continues to be read only as well.

	Okay, I did not notice that the targetm structure now has
constants in addition to functions.  I also misunderstood that sdata
normally does permit symbols which would have been placed in a readonly
section. 

	Is the following patch more what you meant?

Thanks, David


	* target-def.h (TARGET_HAVE_SRODATA_SECTION): New macro.
	* target.h (gcc_target): Add have_srodata_section member.
	* varasm.c (section_category): Add SECCAT_SRODATA.
	(categorize_decl_for_section): Return SECCAT_SRODATA for sdata if
	READONLY_SDATA_SECTION defined.
	(decl_readonly_section_1): True for SECCAT_SRODATA also.
	(default_elf_select_section_1): Map SECCAT_SRODATA to .sdata2.
	(default_unique_section_1): Likewise.

Index: target-def.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/target-def.h,v
retrieving revision 1.30
diff -c -p -r1.30 target-def.h
*** target-def.h	21 Aug 2002 02:41:44 -0000	1.30
--- target-def.h	4 Sep 2002 16:35:00 -0000
*************** Foundation, 59 Temple Place - Suite 330,
*** 118,123 ****
--- 118,127 ----
  #define TARGET_HAVE_TLS false
  #endif
  
+ #ifndef TARGET_HAVE_SRODATA_SECTION
+ #define TARGET_HAVE_SRODATA_SECTION false
+ #endif
+ 
  #ifndef TARGET_ASM_EXCEPTION_SECTION
  #define TARGET_ASM_EXCEPTION_SECTION default_exception_section
  #endif
*************** Foundation, 59 Temple Place - Suite 330,
*** 253,259 ****
    TARGET_STRIP_NAME_ENCODING,			\
    TARGET_HAVE_NAMED_SECTIONS,			\
    TARGET_HAVE_CTORS_DTORS,			\
!   TARGET_HAVE_TLS				\
  }
  
  #include "hooks.h"
--- 257,264 ----
    TARGET_STRIP_NAME_ENCODING,			\
    TARGET_HAVE_NAMED_SECTIONS,			\
    TARGET_HAVE_CTORS_DTORS,			\
!   TARGET_HAVE_TLS,				\
!   TARGET_HAVE_SRODATA_SECTION			\
  }
  
  #include "hooks.h"
Index: target.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/target.h,v
retrieving revision 1.33
diff -c -p -r1.33 target.h
*** target.h	21 Aug 2002 02:41:44 -0000	1.33
--- target.h	4 Sep 2002 16:35:00 -0000
*************** struct gcc_target
*** 262,267 ****
--- 262,270 ----
  
    /* True if thread-local storage is supported.  */
    bool have_tls;
+ 
+   /* True if a small readonly data section is supported.  */
+   bool have_srodata_section;
  };
  
  extern struct gcc_target targetm;
Index: varasm.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/varasm.c,v
retrieving revision 1.304
diff -c -p -r1.304 varasm.c
*** varasm.c	4 Sep 2002 01:56:11 -0000	1.304
--- varasm.c	4 Sep 2002 16:35:01 -0000
*************** enum section_category
*** 4898,4903 ****
--- 4898,4904 ----
    SECCAT_RODATA_MERGE_STR,
    SECCAT_RODATA_MERGE_STR_INIT,
    SECCAT_RODATA_MERGE_CONST,
+   SECCAT_SRODATA,
  
    SECCAT_DATA,
  
*************** categorize_decl_for_section (decl, reloc
*** 4999,5004 ****
--- 5000,5007 ----
      {
        if (ret == SECCAT_BSS)
  	ret = SECCAT_SBSS;
+       else if (targetm.have_srodata_section && ret == SECCAT_RODATA)
+ 	ret = SECCAT_SRODATA;
        else
  	ret = SECCAT_SDATA;
      }
*************** decl_readonly_section_1 (decl, reloc, sh
*** 5026,5031 ****
--- 5029,5035 ----
      case SECCAT_RODATA_MERGE_STR:
      case SECCAT_RODATA_MERGE_STR_INIT:
      case SECCAT_RODATA_MERGE_CONST:
+     case SECCAT_SRODATA:
        return true;
        break;
      default:
*************** default_elf_select_section_1 (decl, relo
*** 5069,5074 ****
--- 5073,5081 ----
      case SECCAT_RODATA_MERGE_CONST:
        mergeable_constant_section (DECL_MODE (decl), align, 0);
        break;
+     case SECCAT_SRODATA:
+       named_section (NULL_TREE, ".sdata2", reloc);
+       break;
      case SECCAT_DATA:
        data_section ();
        break;
*************** default_unique_section_1 (decl, reloc, s
*** 5140,5145 ****
--- 5147,5155 ----
      case SECCAT_RODATA_MERGE_STR_INIT:
      case SECCAT_RODATA_MERGE_CONST:
        prefix = one_only ? ".gnu.linkonce.r." : ".rodata.";
+       break;
+     case SECCAT_SRODATA:
+       prefix = one_only ? ".gnu.linkonce.s2." : ".sdata2.";
        break;
      case SECCAT_DATA:
      case SECCAT_DATA_REL:

Re: [RFC] PowerPC select_section / unique_section

[Richard Henderson]

On Wed, Sep 04, 2002 at 12:40:10PM -0400, David Edelsohn wrote:
> 	* target-def.h (TARGET_HAVE_SRODATA_SECTION): New macro.
> 	* target.h (gcc_target): Add have_srodata_section member.
> 	* varasm.c (section_category): Add SECCAT_SRODATA.
> 	(categorize_decl_for_section): Return SECCAT_SRODATA for sdata if
> 	READONLY_SDATA_SECTION defined.
> 	(decl_readonly_section_1): True for SECCAT_SRODATA also.
> 	(default_elf_select_section_1): Map SECCAT_SRODATA to .sdata2.
> 	(default_unique_section_1): Likewise.

Yes, this is what I meant.  Thanks.


r~

Re: [RFC] PowerPC select_section / unique_section

[Richard Henderson]

On Tue, Sep 03, 2002 at 01:40:19PM -0400, David Edelsohn wrote:
> 	* varasm.c (default_section_type_flags): Append _1 to name with
> 	shlib parameter.  Use original name to call new function with
> 	implicit flag_pic.
> 	(decl_readonly_section): Likewise.
> 	(default_elf_select_section): Likewise.
> 	(default_unique_section): Likewise.
> 	(default_bind_local_p): Likewise.
> 	(categorize_decl_for_section): Add shlib parameter to use in place
> 	of implicit flag_pic.
> 	* output.h: Declare new functions with _1 and shlib argument.

Ok.


r~

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Richard Henderson writes:

>> Do you want all of the default_* functions wrapped around a
>> default_*_1 function instead of adding flag_pic at the call sites?

Richard> I think so, yes.

	How about the following patch?  Then we can move on to integrating
sdata. 

Thanks, David

	* varasm.c (default_section_type_flags): Append _1 to name with
	shlib parameter.  Use original name to call new function with
	implicit flag_pic.
	(decl_readonly_section): Likewise.
	(default_elf_select_section): Likewise.
	(default_unique_section): Likewise.
	(default_bind_local_p): Likewise.
	(categorize_decl_for_section): Add shlib parameter to use in place
	of implicit flag_pic.
	* output.h: Declare new functions with _1 and shlib argument.

Index: varasm.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/varasm.c,v
retrieving revision 1.303
diff -c -p -r1.303 varasm.c
*** varasm.c	21 Aug 2002 02:41:44 -0000	1.303
--- varasm.c	3 Sep 2002 17:29:08 -0000
*************** init_varasm_once ()
*** 4686,4701 ****
     read-only for a const data decl, and writable for a non-const data decl.  */
  
  unsigned int
! default_section_type_flags (decl, name, reloc)
       tree decl;
       const char *name;
       int reloc;
  {
    unsigned int flags;
  
    if (decl && TREE_CODE (decl) == FUNCTION_DECL)
      flags = SECTION_CODE;
!   else if (decl && decl_readonly_section (decl, reloc))
      flags = 0;
    else
      flags = SECTION_WRITE;
--- 4686,4702 ----
     read-only for a const data decl, and writable for a non-const data decl.  */
  
  unsigned int
! default_section_type_flags_1 (decl, name, reloc, shlib)
       tree decl;
       const char *name;
       int reloc;
+      int shlib;
  {
    unsigned int flags;
  
    if (decl && TREE_CODE (decl) == FUNCTION_DECL)
      flags = SECTION_CODE;
!   else if (decl && decl_readonly_section_1 (decl, reloc, shlib))
      flags = 0;
    else
      flags = SECTION_WRITE;
*************** default_section_type_flags (decl, name, 
*** 4725,4730 ****
--- 4726,4740 ----
    return flags;
  }
  
+ unsigned int
+ default_section_type_flags (decl, name, reloc)
+      tree decl;
+      const char *name;
+      int reloc;
+ {
+   return default_section_type_flags_1 (decl, name, reloc, flag_pic);
+ }
+ 
  /* Output assembly to switch to section NAME with attribute FLAGS.
     Four variants for common object file formats.  */
  
*************** enum section_category
*** 4913,4924 ****
    SECCAT_TBSS
  };
  
! static enum section_category categorize_decl_for_section PARAMS ((tree, int));
  
  static enum section_category
! categorize_decl_for_section (decl, reloc)
       tree decl;
       int reloc;
  {
    enum section_category ret;
  
--- 4923,4936 ----
    SECCAT_TBSS
  };
  
! static enum section_category
! categorize_decl_for_section PARAMS ((tree, int, int));
  
  static enum section_category
! categorize_decl_for_section (decl, reloc, shlib)
       tree decl;
       int reloc;
+      int shlib;
  {
    enum section_category ret;
  
*************** categorize_decl_for_section (decl, reloc
*** 4940,4955 ****
  	       || TREE_SIDE_EFFECTS (decl)
  	       || ! TREE_CONSTANT (DECL_INITIAL (decl)))
  	{
! 	  if (flag_pic && (reloc & 2))
  	    ret = SECCAT_DATA_REL;
! 	  else if (flag_pic && reloc)
  	    ret = SECCAT_DATA_REL_LOCAL;
  	  else
  	    ret = SECCAT_DATA;
  	}
!       else if (flag_pic && (reloc & 2))
  	ret = SECCAT_DATA_REL_RO;
!       else if (flag_pic && reloc)
  	ret = SECCAT_DATA_REL_RO_LOCAL;
        else if (flag_merge_constants < 2)
  	/* C and C++ don't allow different variables to share the same
--- 4952,4967 ----
  	       || TREE_SIDE_EFFECTS (decl)
  	       || ! TREE_CONSTANT (DECL_INITIAL (decl)))
  	{
! 	  if (shlib && (reloc & 2))
  	    ret = SECCAT_DATA_REL;
! 	  else if (shlib && reloc)
  	    ret = SECCAT_DATA_REL_LOCAL;
  	  else
  	    ret = SECCAT_DATA;
  	}
!       else if (shlib && (reloc & 2))
  	ret = SECCAT_DATA_REL_RO;
!       else if (shlib && reloc)
  	ret = SECCAT_DATA_REL_RO_LOCAL;
        else if (flag_merge_constants < 2)
  	/* C and C++ don't allow different variables to share the same
*************** categorize_decl_for_section (decl, reloc
*** 4963,4969 ****
      }
    else if (TREE_CODE (decl) == CONSTRUCTOR)
      {
!       if ((flag_pic && reloc)
  	  || TREE_SIDE_EFFECTS (decl)
  	  || ! TREE_CONSTANT (decl))
  	ret = SECCAT_DATA;
--- 4975,4981 ----
      }
    else if (TREE_CODE (decl) == CONSTRUCTOR)
      {
!       if ((shlib && reloc)
  	  || TREE_SIDE_EFFECTS (decl)
  	  || ! TREE_CONSTANT (decl))
  	ret = SECCAT_DATA;
*************** categorize_decl_for_section (decl, reloc
*** 4995,5005 ****
  }
  
  bool
! decl_readonly_section (decl, reloc)
       tree decl;
       int reloc;
  {
!   switch (categorize_decl_for_section (decl, reloc))
      {
      case SECCAT_RODATA:
      case SECCAT_RODATA_MERGE_STR:
--- 5007,5018 ----
  }
  
  bool
! decl_readonly_section_1 (decl, reloc, shlib)
       tree decl;
       int reloc;
+      int shlib;
  {
!   switch (categorize_decl_for_section (decl, reloc, shlib))
      {
      case SECCAT_RODATA:
      case SECCAT_RODATA_MERGE_STR:
*************** decl_readonly_section (decl, reloc)
*** 5013,5027 ****
      }
  }
  
  /* Select a section based on the above categorization.  */
  
  void
! default_elf_select_section (decl, reloc, align)
       tree decl;
       int reloc;
       unsigned HOST_WIDE_INT align;
  {
!   switch (categorize_decl_for_section (decl, reloc))
      {
      case SECCAT_TEXT:
        /* We're not supposed to be called on FUNCTION_DECLs.  */
--- 5026,5049 ----
      }
  }
  
+ bool
+ decl_readonly_section (decl, reloc)
+      tree decl;
+      int reloc;
+ {
+   return decl_readonly_section_1 (decl, reloc, flag_pic);
+ }
+ 
  /* Select a section based on the above categorization.  */
  
  void
! default_elf_select_section_1 (decl, reloc, align, shlib)
       tree decl;
       int reloc;
       unsigned HOST_WIDE_INT align;
+      int shlib;
  {
!   switch (categorize_decl_for_section (decl, reloc, shlib))
      {
      case SECCAT_TEXT:
        /* We're not supposed to be called on FUNCTION_DECLs.  */
*************** default_elf_select_section (decl, reloc,
*** 5077,5096 ****
      }
  }
  
  /* Construct a unique section name based on the decl name and the
     categorization performed above.  */
  
  void
! default_unique_section (decl, reloc)
       tree decl;
       int reloc;
  {
    bool one_only = DECL_ONE_ONLY (decl);
    const char *prefix, *name;
    size_t nlen, plen;
    char *string;
  
!   switch (categorize_decl_for_section (decl, reloc))
      {
      case SECCAT_TEXT:
        prefix = one_only ? ".gnu.linkonce.t." : ".text.";
--- 5099,5128 ----
      }
  }
  
+ void
+ default_elf_select_section (decl, reloc, align)
+      tree decl;
+      int reloc;
+      unsigned HOST_WIDE_INT align;
+ {
+   return default_elf_select_section_1 (decl, reloc, align, flag_pic);
+ }
+ 
  /* Construct a unique section name based on the decl name and the
     categorization performed above.  */
  
  void
! default_unique_section_1 (decl, reloc, shlib)
       tree decl;
       int reloc;
+      int shlib;
  {
    bool one_only = DECL_ONE_ONLY (decl);
    const char *prefix, *name;
    size_t nlen, plen;
    char *string;
  
!   switch (categorize_decl_for_section (decl, reloc, shlib))
      {
      case SECCAT_TEXT:
        prefix = one_only ? ".gnu.linkonce.t." : ".text.";
*************** default_unique_section (decl, reloc)
*** 5140,5145 ****
--- 5172,5185 ----
  }
  
  void
+ default_unique_section (decl, reloc)
+      tree decl;
+      int reloc;
+ {
+   return default_unique_section_1 (decl, reloc, flag_pic);
+ }
+ 
+ void
  default_select_rtx_section (mode, x, align)
       enum machine_mode mode ATTRIBUTE_UNUSED;
       rtx x;
*************** default_strip_name_encoding (str)
*** 5202,5209 ****
     wrt cross-module name binding.  */
  
  bool
! default_binds_local_p (exp)
       tree exp;
  {
    bool local_p;
  
--- 5242,5250 ----
     wrt cross-module name binding.  */
  
  bool
! default_binds_local_p_1 (exp, shlib)
       tree exp;
+      int shlib;
  {
    bool local_p;
  
*************** default_binds_local_p (exp)
*** 5224,5230 ****
      local_p = false;
    /* If PIC, then assume that any global name can be overridden by
       symbols resolved from other modules.  */
!   else if (flag_pic)
      local_p = false;
    /* Uninitialized COMMON variable may be unified with symbols
       resolved from other modules.  */
--- 5265,5271 ----
      local_p = false;
    /* If PIC, then assume that any global name can be overridden by
       symbols resolved from other modules.  */
!   else if (shlib)
      local_p = false;
    /* Uninitialized COMMON variable may be unified with symbols
       resolved from other modules.  */
*************** default_binds_local_p (exp)
*** 5238,5243 ****
--- 5279,5291 ----
      local_p = true;
  
    return local_p;
+ }
+ 
+ bool
+ default_binds_local_p (exp)
+      tree exp;
+ {
+   return default_binds_local_p (exp, flag_pic);
  }
  
  /* Default function to output code that will globalize a label.  A
Index: output.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/output.h,v
retrieving revision 1.109
diff -c -p -r1.109 output.h
*** output.h	21 Aug 2002 02:41:44 -0000	1.109
--- output.h	3 Sep 2002 17:29:08 -0000
*************** extern rtx this_is_asm_operands;
*** 468,473 ****
--- 468,474 ----
  /* Decide whether DECL needs to be in a writable section.
     RELOC is the same as for SELECT_SECTION.  */
  extern bool decl_readonly_section PARAMS ((tree, int));
+ extern bool decl_readonly_section_1 PARAMS ((tree, int, int));
  
  /* User label prefix in effect for this compilation.  */
  extern const char *user_label_prefix;
*************** extern bool named_section_first_declarat
*** 508,513 ****
--- 509,517 ----
  union tree_node;
  extern unsigned int default_section_type_flags PARAMS ((union tree_node *,
  							const char *, int));
+ extern unsigned int default_section_type_flags_1 PARAMS ((union tree_node *,
+ 							  const char *,
+ 							  int, int));
  
  extern void default_no_named_section PARAMS ((const char *, unsigned int));
  extern void default_elf_asm_named_section PARAMS ((const char *, unsigned int));
*************** extern void default_select_section PARAM
*** 530,542 ****
--- 534,550 ----
  					    unsigned HOST_WIDE_INT));
  extern void default_elf_select_section PARAMS ((tree, int,
  						unsigned HOST_WIDE_INT));
+ extern void default_elf_select_section_1 PARAMS ((tree, int,
+ 						  unsigned HOST_WIDE_INT, int));
  extern void default_unique_section PARAMS ((tree, int));
+ extern void default_unique_section_1 PARAMS ((tree, int, int));
  extern void default_select_rtx_section PARAMS ((enum machine_mode, rtx,
  						unsigned HOST_WIDE_INT));
  extern void default_elf_select_rtx_section PARAMS ((enum machine_mode, rtx,
  						    unsigned HOST_WIDE_INT));
  extern const char *default_strip_name_encoding PARAMS ((const char *));
  extern bool default_binds_local_p PARAMS ((tree));
+ extern bool default_binds_local_p_1 PARAMS ((tree, int));
  extern void default_globalize_label PARAMS ((FILE *, const char *));
  
  /* Emit data for vtable gc for GNU binutils.  */

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Geoff Keating writes:

Geoff> Do you need to do this for AIX?  I thought the symbol binding rules
Geoff> for AIX shared libraries were different, they didn't permit overriding
Geoff> a symbol in a shared library.

	Because AIX does support a run-time linking mode, this could be an
issue.  IBM's compilers currently do not change their code generation to
make this easier or more difficult.

	The two issues for both AIX and linuxppc64 are placing certain
types of data in writeable versus read-only sections and the ability to
interpose functions (binds_local_p).

	My current belief is that AIX and linuxppc64 always should default
to PIC and shareable, and that -fpic should only affect binds_local_p to
ensure that symbols can be interposed.  The 64-bit PowerPC SVR4 ABI
already does have a non-PIC mode which GCC does not generate, so having a
non-PIC mode for PIC code would add confusion.  Some of the choice of
sections depends on the PowerPC architecture and how best to utilize it.

	I don't think that any policy change from the current defaults
should be made without a lot more investigation -- which I am doing.

Thanks, David

Re: [RFC] PowerPC select_section / unique_section

[Mark Mitchell]

--On Tuesday, September 03, 2002 10:57:54 AM +0930 Alan Modra 
<amodra@bigpond.net.au> wrote:

> On Mon, Sep 02, 2002 at 08:57:31PM -0400, David Edelsohn wrote:
>> 	To fix the need for GCC to know that it is compiling in PIC mode,
>> the linuxppc64 and AIX targets can:
>>
>> 1) allow flag_pic to be set on the commandline (creating an artificial
>> distinction between PIC and non-PIC object files), or
>
> This is the one we want.  In our case -fpic doesn't mean "position
> independent code", as we're always that sort of PIC.  Instead it just
> means create code for shared library linking semantics.  Note that
> -fpic means both PIC code _and_ shared library code generation on
> other targets.  Which is perhaps unfortunate as they are really two
> separate issues.

So why not fix this?

Add -fshared-lib, and have -fpic turn it on.  Then, test flag_shared_lib
where you mean that, and flag_pic where you mean that.  There's no
compatibility problem for existing code, and on systems where the two
ideas are different, people now have a way of saying which mode they
want.

(You have to notice -fpic -fno_pic which now means something different
than it did before, and warn about that, but that will be a rare case.)

-- 
Mark Mitchell                mark@codesourcery.com
CodeSourcery, LLC            http://www.codesourcery.com

Re: [RFC] PowerPC select_section / unique_section

[Richard Henderson]

On Tue, Sep 03, 2002 at 11:21:18AM -0400, David Edelsohn wrote:
> 	Like the appended patch?

Yes, thanks.

> 	Do you want all of the default_* functions wrapped around a
> default_*_1 function instead of adding flag_pic at the call sites?

I think so, yes.


r~

Re: [RFC] PowerPC select_section / unique_section

[Geoff Keating]

> X-Sieve: cmu-sieve 2.0
> cc: Franz Sirl <Franz.Sirl-kernel@lauterbach.com>,
>    Geoff Keating <geoffk@redhat.com>, gcc-patches@gcc.gnu.org
> Date: Mon, 02 Sep 2002 20:57:31 -0400
> From: David Edelsohn <dje@watson.ibm.com>
> 
> 	To fix the need for GCC to know that it is compiling in PIC mode,
> the linuxppc64 and AIX targets can:

Do you need to do this for AIX?  I thought the symbol binding rules
for AIX shared libraries were different, they didn't permit overriding
a symbol in a shared library.

-- 
- Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Richard Henderson writes:

Richard> I would be willing to extract the bulk of default_binds_local_p
Richard> into a default_binds_local_p_1 that did have a "shlib" parameter,
Richard> and similar for select_section, but I don't want the choice of
Richard> what value of flag_pic to pass to binds_local_p to reside at all
Richard> of the call sites.

	Like the appended patch?

	Do you want all of the default_* functions wrapped around a
default_*_1 function instead of adding flag_pic at the call sites?

Thanks, David


	* varasm.c (default_binds_local_p): Rename as
	default_binds_local_p_1 with shlib parameter.  Use original name
	to call new function with flag_pic.

Index: varasm.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/varasm.c,v
retrieving revision 1.303
diff -c -p -r1.303 varasm.c
*** varasm.c	21 Aug 2002 02:41:44 -0000	1.303
--- varasm.c	3 Sep 2002 15:15:36 -0000
*************** default_strip_name_encoding (str)
*** 5202,5209 ****
     wrt cross-module name binding.  */
  
  bool
! default_binds_local_p (exp)
       tree exp;
  {
    bool local_p;
  
--- 5205,5213 ----
     wrt cross-module name binding.  */
  
  bool
! default_binds_local_p_1 (exp, shlib)
       tree exp;
+      int shlib;
  {
    bool local_p;
  
*************** default_binds_local_p (exp)
*** 5224,5230 ****
      local_p = false;
    /* If PIC, then assume that any global name can be overridden by
       symbols resolved from other modules.  */
!   else if (flag_pic)
      local_p = false;
    /* Uninitialized COMMON variable may be unified with symbols
       resolved from other modules.  */
--- 5228,5234 ----
      local_p = false;
    /* If PIC, then assume that any global name can be overridden by
       symbols resolved from other modules.  */
!   else if (shlib)
      local_p = false;
    /* Uninitialized COMMON variable may be unified with symbols
       resolved from other modules.  */
*************** default_binds_local_p (exp)
*** 5238,5243 ****
--- 5242,5254 ----
      local_p = true;
  
    return local_p;
+ }
+ 
+ bool
+ default_binds_local_p (exp)
+      tree exp;
+ {
+   return default_binds_local_p (exp, flag_pic);
  }
  
  /* Default function to output code that will globalize a label.  A
Index: output.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/output.h,v
retrieving revision 1.109
diff -c -p -r1.109 output.h
*** output.h	21 Aug 2002 02:41:44 -0000	1.109
--- output.h	3 Sep 2002 15:15:37 -0000
*************** extern void default_elf_select_rtx_secti
*** 537,542 ****
--- 537,543 ----
  						    unsigned HOST_WIDE_INT));
  extern const char *default_strip_name_encoding PARAMS ((const char *));
  extern bool default_binds_local_p PARAMS ((tree));
+ extern bool default_binds_local_p_1 PARAMS ((tree, int));
  extern void default_globalize_label PARAMS ((FILE *, const char *));
  
  /* Emit data for vtable gc for GNU binutils.  */

Re: [RFC] PowerPC select_section / unique_section

[Richard Henderson]

On Mon, Sep 02, 2002 at 11:19:11PM -0400, David Edelsohn wrote:
> Jeff> Some targets.  Unless I'm very mistaken, all code is PIC on alpha,
> Jeff> so that -fPIC has little effect apart from binds_local_p.

Well, it also changes what sections data with relocations are placed in.
But I think you underestimate what effect binds_local_p has -- variables
that are known to be local are addressed with gp-relative relocations
instead of through the got, calls within the same UOT use bsr, etc.

> Jeff> Irrespective of PPC, wouldn't it be a desirable to have uniformity
> Jeff> across GNU/Linux targets over this issue?

Yes.  But aside from PPC, I think we largely have that already.

> 	Which seems like a good argument for default_binds_local_p and
> others using a "pic" function parameter instead of grabbing the global
> flag_pic.  This would allow -fpic/-fPIC to affect targetm.binds_local_p
> while not affecting the other functions.

I don't see how that follows.  In fact, I think that would simply
be confusing to the many places that want to call binds_local_p.

I would be willing to extract the bulk of default_binds_local_p
into a default_binds_local_p_1 that did have a "shlib" parameter,
and similar for select_section, but I don't want the choice of
what value of flag_pic to pass to binds_local_p to reside at all
of the call sites.



r~

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Jeff Sturm writes:

Jeff> Some targets.  Unless I'm very mistaken, all code is PIC on alpha, so that
Jeff> -fPIC has little effect apart from binds_local_p.

Jeff> Irrespective of PPC, wouldn't it be a desirable to have uniformity across
Jeff> GNU/Linux targets over this issue?

	Which seems like a good argument for default_binds_local_p and
others using a "pic" function parameter instead of grabbing the global
flag_pic.  This would allow -fpic/-fPIC to affect targetm.binds_local_p
while not affecting the other functions.

	For instance, rs6000_override_options could reset the GCC global
flag_pic to zero while remembering the original value privately and then
using that value to affect binds_local_p.

	These uses of flag_pic seem like orthogonal decisions to me and
ports should have finer-grained control only available if binds_local_p
and select_section accept arguments.

Thanks, David

Re: [RFC] PowerPC select_section / unique_section

[Richard Henderson]

On Mon, Sep 02, 2002 at 09:49:11PM -0400, David Edelsohn wrote:
> 	GCC always accesses globals through the TOC and always calls
> public functions through glue, so symbols always can be overridden -- at
> least until the recent binds_local_p changes.

This is actually incorrect.  Until the binds_local_p changes, various
parts of gcc did various checks (or not), somewhat at random.  The
binds_local_p change "merely" centralizes the logic.


r~

Re: [RFC] PowerPC select_section / unique_section

[Jeff Sturm]

On Tue, 3 Sep 2002, Alan Modra wrote:
> Note that -fpic means both PIC code _and_ shared library code generation
> on other targets.

Some targets.  Unless I'm very mistaken, all code is PIC on alpha, so that
-fPIC has little effect apart from binds_local_p.

Irrespective of PPC, wouldn't it be a desirable to have uniformity across
GNU/Linux targets over this issue?

Jeff

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Alan Modra writes:

Alan> You may make the decision that AIX gcc doesn't need these
Alan> optimizations, but please don't hobble PowerPC64 Linux.

	Please tone it down and stop making this personal.

	This type of decision needs input from others, including other
experts on calling conventions and compilers within IBM.

David

Re: [RFC] PowerPC select_section / unique_section

[Alan Modra]

On Mon, Sep 02, 2002 at 09:49:11PM -0400, David Edelsohn wrote:
> 	It is not obvious that GCC should behave differently for shared
> libraries and regular object files when targeting ABI_AIX. 

I don't know how to make it any more obvious to you.  Do you disagree
with:

1) Current mainline powerpc64-linux gcc, in the face of inlining and
   const function folding optimizations, is broken when building
   shared libraries.

2) Therefore these optimizations must be turned off when building
   shared libraries.

3) If shared and regular objects are to be built the same, then these
   optimizations must be turned off for regular objects too.

You may make the decision that AIX gcc doesn't need these
optimizations, but please don't hobble PowerPC64 Linux.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Alan Modra writes:

Alan> 3) and 4) don't address how gcc should be told to generate code for
Alan> shared libraries.  You seem to be assuming some flag other than -fpic
Alan> would be used.

	I am not assuming any additional flags.

	The question is whether we ever need to distinguish between code
generation for shared libraries and code generation for regular object
files.  AIX compilers never make any distinction.

	AIX default to tight binding within shared objects, but AIX 4.2
and above added the runtime-linking flag to allow SVR4-type symbol
overriding.  No changes have been necessary to IBM's compilers or to GCC
to support this.  There is no "prepare for runtime-linking" compiler
option.

	GCC always accesses globals through the TOC and always calls
public functions through glue, so symbols always can be overridden -- at
least until the recent binds_local_p changes.

	It is not obvious that GCC should behave differently for shared
libraries and regular object files when targeting ABI_AIX. 

David

Re: [RFC] PowerPC select_section / unique_section

[Alan Modra]

On Mon, Sep 02, 2002 at 08:57:31PM -0400, David Edelsohn wrote:
> 	To fix the need for GCC to know that it is compiling in PIC mode,
> the linuxppc64 and AIX targets can:
> 
> 1) allow flag_pic to be set on the commandline (creating an artificial
> distinction between PIC and non-PIC object files), or

This is the one we want.  In our case -fpic doesn't mean "position
independent code", as we're always that sort of PIC.  Instead it just
means create code for shared library linking semantics.  Note that
-fpic means both PIC code _and_ shared library code generation on
other targets.  Which is perhaps unfortunate as they are really two
separate issues.

> 2) uniformly set flag_pic (creating problems because of the other uses of
> flag_pic in the compiler), or

No, because then you lose optimization opportunities, as rth pointed
out.

> 3) set and unset flag_pic around calls to the select section and
> default_binds_local_p functions, or
> 
> 4) modify the default select section and default_binds_local_p functions
> to accept a "pic" argument (and override targetm.binds_local_p for
> PowerPC).
> 
> 	I prefer option 4 to allow finer-grained control, allow greater
> compiler optimization opportunities, and use better programming practice.

3) and 4) don't address how gcc should be told to generate code for
shared libraries.  You seem to be assuming some flag other than -fpic
would be used.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

	To fix the need for GCC to know that it is compiling in PIC mode,
the linuxppc64 and AIX targets can:

1) allow flag_pic to be set on the commandline (creating an artificial
distinction between PIC and non-PIC object files), or

2) uniformly set flag_pic (creating problems because of the other uses of
flag_pic in the compiler), or

3) set and unset flag_pic around calls to the select section and
default_binds_local_p functions, or

4) modify the default select section and default_binds_local_p functions
to accept a "pic" argument (and override targetm.binds_local_p for
PowerPC).

	I prefer option 4 to allow finer-grained control, allow greater
compiler optimization opportunities, and use better programming practice.

David

Re: [RFC] PowerPC select_section / unique_section

[Alan Modra]

On Mon, Sep 02, 2002 at 07:51:07PM -0400, David Edelsohn wrote:
> Alan> We (linux people) need a flag to say "build for a shared library".
> 
> 	You have not justified this statement.

See http://gcc.gnu.org/ml/gcc-patches/2002-08/msg01821.html

Note "global functions may be overridden".  A silly example, but gcc
cannot optimize the calls to foo in the following if this code appears
in an ELF shared library, as the actual foo called might do something
completely different, like open an emacs window.

int foo (void)
{
  return 0;
}

int bar (void)
{
  return foo () + foo ();
}

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

	Again, no one has said what is incorrect about about GCC's current
code generation, other than Richard's report about inlining and -O3.  Not
what is inefficient, but what produces incorrect code.

	GCC targeted for AIX and linuxppc64 currently always should
produce code appropriate for shared libraries, with the most recent
changes, other than the problem reported above.  If GCC should produce
different code without -fpic, that is a different question and not a bug.

Alan> We (linux people) need a flag to say "build for a shared library".

	You have not justified this statement.

	All I have seen so far is a bunch of proposed patches and
assumptions without analysis (other than Richard).  When I see a report of
an actual problem, then we can discuss how to solve it, not patches in
search of a bug.

David

Re: [RFC] PowerPC select_section / unique_section

[Alan Modra]

You can simply allow -fpic to be accepted by ppc64 gcc, and qualify
all the occurrences of flag_pic in rs6000/* with a test on
DEFAULT_ABI.  No special private functions, no modifications to
varasm.c, and as a bonus, a reduction in the size of rs6000.o.
We (linux people) need a flag to say "build for a shared library".
Too many packages use -fpic for this purpose to warrant choosing
some other flag for PowerPC64.

As I said before, I have a patch that does this but David curtly
told me, without explanation, "Don't bother" posting it.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Richard Henderson writes:

Richard> The inliner will use it in a moment as well.  There's an outstanding
Richard> bug wrt -O3 that inlines functions that it shouldn't.

	Either this is a bug which needs to be fixed in GCC 3.2/3.3 or can
wait until GCC 3.4.

	If this needs to be fixed in GCC 3.2/3.3, then either it can be
fixed by tweaking the current PowerPC-specific functions or we need to
extend the default functionality in varasm.c so that the PowerPC port can
tie into it.

	I am happy to help extend the current varasm.c infrastructure for
the PowerPC port to use in the GCC 3.3 release.  I do not believe that
creating yet another different, private set of functions in rs6000.c for
the PowerPC port (even in the short term) is necessary or a good strategy.

David

Re: [RFC] PowerPC select_section / unique_section

[Geoff Keating]

> Date: Fri, 30 Aug 2002 18:09:27 -0700
> From: Richard Henderson <rth@redhat.com>

> On Sat, Aug 31, 2002 at 09:19:30AM +0930, Alan Modra wrote:
> > The PowerPC back-end code doesn't use binds_local_p, but
> > mark_constant_function does.
> 
> The inliner will use it in a moment as well.  There's an outstanding
> bug wrt -O3 that inlines functions that it shouldn't.

FYI, I tried to fix this the obvious way,

Index: gcc/gcc/c-decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-decl.c,v
retrieving revision 1.349
diff -p -u -p -r1.349 c-decl.c
--- gcc/gcc/c-decl.c    21 Aug 2002 16:31:34 -0000      1.349
+++ gcc/gcc/c-decl.c    31 Aug 2002 01:33:02 -0000
@@ -4546,9 +4546,14 @@ grokdeclarator (declarator, declspecs, d
              }
          }
        /* If -finline-functions, assume it can be inlined.  This does
-          two things: let the function be deferred until it is actually
-          needed, and let dwarf2 know that the function is inlinable.  */
-       else if (flag_inline_trees == 2 && initialized)
+          two things: let the function be deferred until it is
+          actually needed, and let dwarf2 know that the function is
+          inlinable.  Don't inline anything that might not actually
+          be this function from this file---this is done here because
+          if the user explicitly specifies 'inline' then we want to
+          inline anyway.  */
+       else if (flag_inline_trees == 2 && initialized
+                && (*targetm.binds_local_p) (decl))
          { 
            DECL_INLINE (decl) = 1;
            DECL_DECLARED_INLINE_P (decl) = 0;

(and equivalently for C++) but this doesn't allow globally-visible
functions to be inlined when not -fpic on x86.  Possibly this means
binds_local_p is wrong on x86.

-- 
- Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>

Re: [RFC] PowerPC select_section / unique_section

[Richard Henderson]

On Sat, Aug 31, 2002 at 09:19:30AM +0930, Alan Modra wrote:
> The PowerPC back-end code doesn't use binds_local_p, but
> mark_constant_function does.

The inliner will use it in a moment as well.  There's an outstanding
bug wrt -O3 that inlines functions that it shouldn't.


r~

Re: [RFC] PowerPC select_section / unique_section

[Alan Modra]

On Fri, Aug 30, 2002 at 11:27:25AM -0400, David Edelsohn wrote:
> >>>>> Alan Modra writes:
> 
> Alan> Uh, oh.  If I understand binds_local_p and mark_constant_function
> Alan> correctly, setting flag_pic is actually a bug-fix when compiling
> Alan> powerpc64-linux shared libs.  We have the standard ELF binding of
> Alan> global syms.  That is, global functions may be overridden by functions
> Alan> in another shared library or by the main application.
> 
> Alan> So powerpc64-linux-gcc should allow -fpic/PIC to twiddle flag_pic for
> Alan> binds_local_p, and users should set -fPIC when compiling shared libs
> Alan> as is common on other ELF targets.  We could use another flag, because
> Alan> like that annoying rs6000.c warning says "all code is position
> Alan> independent" on ppc64, but that would make powerpc64-linux just that
> Alan> more odd.  Lots of packages set -fPIC to mean "compile me code for a
> Alan> shared library".
> 
> 	This is what the patch that I applied to both gcc-3.2 and the
> trunk already does, without utilizing the generic infrastructure.  The
> PowerPC port currently does not use the targetm.binds_local_p.  We can
> discuss evolving to the generic infrastructure for GCC 3.4.

The PowerPC back-end code doesn't use binds_local_p, but
mark_constant_function does.

I think I'm correct in claiming that powerpc64-linux-gcc lacks a way
to say "I want this code compiled for a shared library;  Don't try to
analyze global functions for pure/const as the function in this file
may not be the one called at runtime."

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Franz Sirl writes:

Franz> What might be acceptable for 3.3 still is the appended patch which 
Franz> basically just copies the varasm routines to rs6000.c and adds SDATA2 and 
Franz> AIXELF PIC handling. Briefly tested on powerpc-linux-gnu without 
Franz> regressions. Can the AIXELF people give it a try?

	No, this is not acceptable for GCC 3.3.  This is not a bugfix.

	For GCC 3.4 we can explore merging the PowerPC functionality in
with the default implementation in varasm.c -- either with an explicit
"pic" argument or modifying flag_pic around the calls. 

	None of the larger, intrusive patches are open for discussion for
GCC 3.3.

David

Re: [RFC] PowerPC select_section / unique_section

[Franz Sirl]

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

At 20:15 30.08.2002, David Edelsohn wrote:
>At 17:27 30.08.2002, David Edelsohn wrote:
>> >>>>> Alan Modra writes:
>>
>>Alan> Uh, oh.  If I understand binds_local_p and mark_constant_function
>>Alan> correctly, setting flag_pic is actually a bug-fix when compiling
>>Alan> powerpc64-linux shared libs.  We have the standard ELF binding of
>>Alan> global syms.  That is, global functions may be overridden by functions
>>Alan> in another shared library or by the main application.
>>
>>Alan> So powerpc64-linux-gcc should allow -fpic/PIC to twiddle flag_pic for
>>Alan> binds_local_p, and users should set -fPIC when compiling shared libs
>>Alan> as is common on other ELF targets.  We could use another flag, because
>>Alan> like that annoying rs6000.c warning says "all code is position
>>Alan> independent" on ppc64, but that would make powerpc64-linux just that
>>Alan> more odd.  Lots of packages set -fPIC to mean "compile me code for a
>>Alan> shared library".
>>
>>         This is what the patch that I applied to both gcc-3.2 and the
>>trunk already does, without utilizing the generic infrastructure.  The
>>PowerPC port currently does not use the targetm.binds_local_p.  We can
>>discuss evolving to the generic infrastructure for GCC 3.4.
>
>What might be acceptable for 3.3 still is the appended patch which 
>basically just copies the varasm routines to rs6000.c and adds SDATA2 and 
>AIXELF PIC handling. Briefly tested on powerpc-linux-gnu without 
>regressions. Can the AIXELF people give it a try? Sorry, no changelog yet, 
>I'm in a hurry right now.

Argh, forgot to remove the define_constant hunks, here the corrected patch.

And one comment, while working on this it occurred to me it's not that easy 
to re-use the varasm code for 3.4 with the current structure, even if I add 
a target hook for categorize_decl_for_section as suggested. The aborts in 
the generic functions make re-use difficult...

Franz.

[-- Attachment #2: gcc-ppcsections-1a.patch --]
[-- Type: application/octet-stream, Size: 13732 bytes --]

Index: gcc/config/rs6000/rs6000.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/rs6000/rs6000.c,v
retrieving revision 1.368
diff -u -p -r1.368 rs6000.c
--- gcc/config/rs6000/rs6000.c	23 Aug 2002 18:02:22 -0000	1.368
+++ gcc/config/rs6000/rs6000.c	30 Aug 2002 18:08:39 -0000
@@ -196,9 +196,11 @@ static unsigned int rs6000_elf_section_t
 							   int));
 static void rs6000_elf_asm_out_constructor PARAMS ((rtx, int));
 static void rs6000_elf_asm_out_destructor PARAMS ((rtx, int));
-static void rs6000_elf_select_section PARAMS ((tree, int,
-						 unsigned HOST_WIDE_INT));
-static void rs6000_elf_unique_section PARAMS ((tree, int));
+static bool rs6000_in_small_data_p PARAMS ((tree));
+
+void rs6000_elf_select_section PARAMS ((tree, int,
+					unsigned HOST_WIDE_INT));
+void rs6000_elf_unique_section PARAMS ((tree, int));
 static void rs6000_elf_select_rtx_section PARAMS ((enum machine_mode, rtx,
 						   unsigned HOST_WIDE_INT));
 static void rs6000_elf_encode_section_info PARAMS ((tree, int));
@@ -308,7 +310,8 @@ static const char alt_reg_names[][8] =
 #define TARGET_ATTRIBUTE_TABLE rs6000_attribute_table
 #undef TARGET_SET_DEFAULT_TYPE_ATTRIBUTES
 #define TARGET_SET_DEFAULT_TYPE_ATTRIBUTES rs6000_set_default_type_attributes
-
+#undef TARGET_IN_SMALL_DATA_P
+#define TARGET_IN_SMALL_DATA_P rs6000_in_small_data_p
 #undef TARGET_ASM_ALIGNED_DI_OP
 #define TARGET_ASM_ALIGNED_DI_OP DOUBLE_INT_ASM_OP
 
@@ -12416,137 +12419,293 @@ rs6000_elf_select_rtx_section (mode, x, 
     default_elf_select_rtx_section (mode, x, align);
 }
 
-/* A C statement or statements to switch to the appropriate
-   section for output of DECL.  DECL is either a `VAR_DECL' node
-   or a constant of some sort.  RELOC indicates whether forming
-   the initial value of DECL requires link-time relocations.  */
 
-static void
-rs6000_elf_select_section (decl, reloc, align)
+static bool
+rs6000_in_small_data_p (decl)
      tree decl;
-     int reloc;
-     unsigned HOST_WIDE_INT align ATTRIBUTE_UNUSED;
 {
-  int size = int_size_in_bytes (TREE_TYPE (decl));
-  bool needs_sdata;
-  bool readonly;
-  static void (* const sec_funcs[4]) PARAMS ((void)) = {
-    &readonly_data_section,
-    &sdata2_section,
-    &data_section,
-    &sdata_section
-  };
+  /* We want to merge strings, so we never consider them small data.  */
+  if (TREE_CODE (decl) == STRING_CST)
+    return false;
   
-  needs_sdata = (size > 0 
-		 && size <= g_switch_value
-		 && rs6000_sdata != SDATA_NONE
-		 && (rs6000_sdata != SDATA_DATA || TREE_PUBLIC (decl)));
+  if (TREE_CODE (decl) == VAR_DECL && DECL_SECTION_NAME (decl))
+    {
+      const char *section = TREE_STRING_POINTER (DECL_SECTION_NAME (decl));
+      if (strcmp (section, ".sdata") == 0
+	  || strcmp (section, ".sdata2") == 0
+	  || strcmp (section, ".sbss") == 0)
+	return true;
+    }
+  else
+    {
+      int size = int_size_in_bytes (TREE_TYPE (decl));
+      return (size > 0
+	      && size <= g_switch_value
+	      && rs6000_sdata != SDATA_NONE
+	      && (rs6000_sdata != SDATA_DATA || TREE_PUBLIC (decl)));
+    }
+}
 
-  if (TREE_CODE (decl) == STRING_CST)
-    readonly = !flag_writable_strings;
+/* A helper function for default_elf_select_section and
+   default_elf_unique_section.  Categorizes the DECL.  */
+
+enum rs6000_section_category
+{
+  SECCAT_TEXT,
+
+  SECCAT_RODATA,
+  SECCAT_RODATA_MERGE_STR,
+  SECCAT_RODATA_MERGE_STR_INIT,
+  SECCAT_RODATA_MERGE_CONST,
+
+  SECCAT_DATA,
+
+  /* To optimize loading of shared programs, define following subsections
+     of data section:
+        _REL    Contains data that has relocations, so they get grouped
+                together and dynamic linker will visit fewer pages in memory.
+        _RO     Contains data that is otherwise read-only.  This is useful
+                with prelinking as most relocations won't be dynamically
+                linked and thus stay read only.
+        _LOCAL  Marks data containing relocations only to local objects.
+                These relocations will get fully resolved by prelinking.  */
+  SECCAT_DATA_REL,
+  SECCAT_DATA_REL_LOCAL,
+  SECCAT_DATA_REL_RO,
+  SECCAT_DATA_REL_RO_LOCAL,
+
+  SECCAT_SDATA,
+  SECCAT_SDATA2,
+  SECCAT_TDATA,
+
+  SECCAT_BSS,
+  SECCAT_SBSS,
+  SECCAT_TBSS
+};
+
+static enum rs6000_section_category rs6000_categorize_decl_for_section PARAMS ((tree, int, int));
+
+static enum rs6000_section_category
+rs6000_categorize_decl_for_section (decl, reloc, pic)
+     tree decl;
+     int reloc;
+     int pic;
+{
+  enum rs6000_section_category ret;
+
+  if (TREE_CODE (decl) == FUNCTION_DECL)
+    return SECCAT_TEXT;
+  else if (TREE_CODE (decl) == STRING_CST)
+    {
+      if (flag_writable_strings)
+        return SECCAT_DATA;
+      else
+        return SECCAT_RODATA_MERGE_STR;
+    }
   else if (TREE_CODE (decl) == VAR_DECL)
-    readonly = (!((flag_pic || DEFAULT_ABI == ABI_AIX) && reloc)
-		&& TREE_READONLY (decl)
-		&& !TREE_SIDE_EFFECTS (decl)
-		&& DECL_INITIAL (decl)
-		&& DECL_INITIAL (decl) != error_mark_node
-		&& TREE_CONSTANT (DECL_INITIAL (decl)));
+    {
+      if (DECL_INITIAL (decl) == NULL
+          || DECL_INITIAL (decl) == error_mark_node)
+        ret = SECCAT_BSS;
+      else if (! TREE_READONLY (decl)
+               || TREE_SIDE_EFFECTS (decl)
+               || ! TREE_CONSTANT (DECL_INITIAL (decl)))
+        {
+          if (pic && (reloc & 2))
+            ret = SECCAT_DATA_REL;
+          else if (pic && reloc)
+            ret = SECCAT_DATA_REL_LOCAL;
+          else
+            ret = SECCAT_DATA;
+        }
+      else if (pic && (reloc & 2))
+        ret = SECCAT_DATA_REL_RO;
+      else if (pic && reloc)
+        ret = SECCAT_DATA_REL_RO_LOCAL;
+      else if (flag_merge_constants < 2)
+        /* C and C++ don't allow different variables to share the same
+           location.  -fmerge-all-constants allows even that (at the
+           expense of not conforming).  */
+        ret = SECCAT_RODATA;
+      else if (TREE_CODE (DECL_INITIAL (decl)) == STRING_CST)
+        ret = SECCAT_RODATA_MERGE_STR_INIT;
+      else
+        ret = SECCAT_RODATA_MERGE_CONST;
+    }
   else if (TREE_CODE (decl) == CONSTRUCTOR)
-    readonly = (!((flag_pic || DEFAULT_ABI == ABI_AIX) && reloc)
-		&& !TREE_SIDE_EFFECTS (decl)
-		&& TREE_CONSTANT (decl));
+    {
+      if ((pic && reloc)
+          || TREE_SIDE_EFFECTS (decl)
+          || ! TREE_CONSTANT (decl))
+        ret = SECCAT_DATA;
+      else
+        ret = SECCAT_RODATA;
+    }
   else
-    readonly = !((flag_pic || DEFAULT_ABI == ABI_AIX) && reloc);
+    ret = SECCAT_RODATA;
 
-  if (needs_sdata && rs6000_sdata != SDATA_EABI)
-    readonly = false;
-  
-  (*sec_funcs[(readonly ? 0 : 2) + (needs_sdata ? 1 : 0)])();
+  /* There are no read-only thread-local sections.  */
+  if (TREE_CODE (decl) == VAR_DECL && DECL_THREAD_LOCAL (decl))
+    {
+      if (ret == SECCAT_BSS)
+        ret = SECCAT_TBSS;
+      else
+        ret = SECCAT_TDATA;
+    }
+
+  /* If the target uses small data sections, select it.  */
+  else if ((*targetm.in_small_data_p) (decl))
+    {
+      if (ret == SECCAT_BSS)
+        ret = SECCAT_SBSS;
+      else
+        ret = SECCAT_SDATA;
+    }
+
+  /* With EABI we put small readonly data into .sdata2.  */
+  if (ret == SECCAT_RODATA && rs6000_sdata == SDATA_EABI
+      && (*targetm.in_small_data_p) (decl))
+    ret = SECCAT_SDATA2;
+
+  return ret;
 }
 
-/* A C statement to build up a unique section name, expressed as a
-   STRING_CST node, and assign it to DECL_SECTION_NAME (decl).
-   RELOC indicates whether the initial value of EXP requires
-   link-time relocations.  If you do not define this macro, GCC will use
-   the symbol name prefixed by `.' as the section name.  Note - this
-   macro can now be called for uninitialized data items as well as
-   initialised data and functions.  */
+/* Select a section based on the above categorization.  */
 
-static void
+void
+rs6000_elf_select_section (decl, reloc, align)
+     tree decl;
+     int reloc;
+     unsigned HOST_WIDE_INT align;
+{
+  bool pic = flag_pic || DEFAULT_ABI == ABI_AIX;
+
+  switch (rs6000_categorize_decl_for_section (decl, reloc, pic))
+    {
+    case SECCAT_TEXT:
+      /* We're not supposed to be called on FUNCTION_DECLs.  */
+      abort ();
+    case SECCAT_RODATA:
+      readonly_data_section ();
+      break;
+    case SECCAT_RODATA_MERGE_STR:
+      mergeable_string_section (decl, align, 0);
+      break;
+    case SECCAT_RODATA_MERGE_STR_INIT:
+      mergeable_string_section (DECL_INITIAL (decl), align, 0);
+      break;
+    case SECCAT_RODATA_MERGE_CONST:
+      mergeable_constant_section (DECL_MODE (decl), align, 0);
+      break;
+    case SECCAT_DATA:
+      data_section ();
+      break;
+    case SECCAT_DATA_REL:
+      named_section (NULL_TREE, ".data.rel", reloc);
+      break;
+    case SECCAT_DATA_REL_LOCAL:
+      named_section (NULL_TREE, ".data.rel.local", reloc);
+      break;
+    case SECCAT_DATA_REL_RO:
+      named_section (NULL_TREE, ".data.rel.ro", reloc);
+      break;
+    case SECCAT_DATA_REL_RO_LOCAL:
+      named_section (NULL_TREE, ".data.rel.ro.local", reloc);
+      break;
+    case SECCAT_SDATA:
+      named_section (NULL_TREE, ".sdata", reloc);
+      break;
+    case SECCAT_SDATA2:
+      named_section (NULL_TREE, ".sdata2", reloc);
+      break;
+    case SECCAT_TDATA:
+      named_section (NULL_TREE, ".tdata", reloc);
+      break;
+    case SECCAT_BSS:
+#ifdef BSS_SECTION_ASM_OP
+      bss_section ();
+#else
+      named_section (NULL_TREE, ".bss", reloc);
+#endif
+      break;
+    case SECCAT_SBSS:
+      named_section (NULL_TREE, ".sbss", reloc);
+      break;
+    case SECCAT_TBSS:
+      named_section (NULL_TREE, ".tbss", reloc);
+      break;
+    default:
+      abort ();
+    }
+}
+
+/* Construct a unique section name based on the decl name and the
+   categorization performed above.  */
+
+void
 rs6000_elf_unique_section (decl, reloc)
      tree decl;
      int reloc;
 {
-  int len;
-  int sec;
-  const char *name;
+  bool one_only = DECL_ONE_ONLY (decl);
+  bool pic = flag_pic || DEFAULT_ABI == ABI_AIX;
+  const char *prefix, *name;
+  size_t nlen, plen;
   char *string;
-  const char *prefix;
 
-  static const char *const prefixes[7][2] =
-  {
-    { ".rodata.", ".gnu.linkonce.r." },
-    { ".sdata2.", ".gnu.linkonce.s2." },
-    { ".data.",   ".gnu.linkonce.d." },
-    { ".sdata.",  ".gnu.linkonce.s." },
-    { ".bss.",    ".gnu.linkonce.b." },
-    { ".sbss.",   ".gnu.linkonce.sb." },
-    { ".text.",   ".gnu.linkonce.t." }
-  };
-
-  if (TREE_CODE (decl) == FUNCTION_DECL)
-    sec = 6;
-  else
+  switch (rs6000_categorize_decl_for_section (decl, reloc, pic))
     {
-      bool readonly;
-      bool needs_sdata;
-      int size;
-
-      if (TREE_CODE (decl) == STRING_CST)
-	readonly = !flag_writable_strings;
-      else if (TREE_CODE (decl) == VAR_DECL)
-	readonly = (!((flag_pic || DEFAULT_ABI == ABI_AIX) && reloc)
-		    && TREE_READONLY (decl)
-		    && !TREE_SIDE_EFFECTS (decl)
-		    && TREE_CONSTANT (DECL_INITIAL (decl)));
-      else
-	readonly = !((flag_pic || DEFAULT_ABI == ABI_AIX) && reloc);
-
-      size = int_size_in_bytes (TREE_TYPE (decl));
-      needs_sdata = (size > 0 
-		     && size <= g_switch_value
-		     && rs6000_sdata != SDATA_NONE
-		     && (rs6000_sdata != SDATA_DATA || TREE_PUBLIC (decl)));
-
-      if (DECL_INITIAL (decl) == NULL
-	  || DECL_INITIAL (decl) == error_mark_node)
-	sec = 4;
-      else if (!readonly)
-	sec = 2;
-      else
-	sec = 0;
-
-      if (needs_sdata)
-	{
-	  /* .sdata2 is only for EABI.  */
-	  if (sec == 0 && rs6000_sdata != SDATA_EABI)
-	    sec = 2;
-	  sec += 1;
-	}
+    case SECCAT_TEXT:
+      prefix = one_only ? ".gnu.linkonce.t." : ".text.";
+      break;
+    case SECCAT_RODATA:
+    case SECCAT_RODATA_MERGE_STR:
+    case SECCAT_RODATA_MERGE_STR_INIT:
+    case SECCAT_RODATA_MERGE_CONST:
+      prefix = one_only ? ".gnu.linkonce.r." : ".rodata.";
+      break;
+    case SECCAT_DATA:
+    case SECCAT_DATA_REL:
+    case SECCAT_DATA_REL_LOCAL:
+    case SECCAT_DATA_REL_RO:
+    case SECCAT_DATA_REL_RO_LOCAL:
+      prefix = one_only ? ".gnu.linkonce.d." : ".data.";
+      break;
+    case SECCAT_SDATA:
+      prefix = one_only ? ".gnu.linkonce.s." : ".sdata.";
+      break;
+    case SECCAT_SDATA2:
+      prefix = one_only ? ".gnu.linkonce.s2." : ".sdata2.";
+      break;
+    case SECCAT_BSS:
+      prefix = one_only ? ".gnu.linkonce.b." : ".bss.";
+      break;
+    case SECCAT_SBSS:
+      prefix = one_only ? ".gnu.linkonce.sb." : ".sbss.";
+      break;
+    case SECCAT_TDATA:
+      prefix = one_only ? ".gnu.linkonce.td." : ".tdata.";
+      break;
+    case SECCAT_TBSS:
+      prefix = one_only ? ".gnu.linkonce.tb." : ".tbss.";
+      break;
+    default:
+      abort ();
     }
+  plen = strlen (prefix);
 
   name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
-  name = (*targetm.strip_name_encoding) (name);
-  prefix = prefixes[sec][DECL_ONE_ONLY (decl)];
-  len    = strlen (name) + strlen (prefix);
-  string = alloca (len + 1);
-  
-  sprintf (string, "%s%s", prefix, name);
-  
-  DECL_SECTION_NAME (decl) = build_string (len, string);
+  name = (* targetm.strip_name_encoding) (name);
+  nlen = strlen (name);
+
+  string = alloca (nlen + plen + 1);
+  memcpy (string, prefix, plen);
+  memcpy (string + plen, name, nlen + 1);
+
+  DECL_SECTION_NAME (decl) = build_string (nlen + plen, string);
 }
 
-\f
+
 /* If we are referencing a function that is static or is known to be
    in this file, make the SYMBOL_REF special.  We can use this to indicate
    that we can branch to this function without emitting a no-op after the

Re: [RFC] PowerPC select_section / unique_section

[Franz Sirl]

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

At 17:27 30.08.2002, David Edelsohn wrote:
> >>>>> Alan Modra writes:
>
>Alan> Uh, oh.  If I understand binds_local_p and mark_constant_function
>Alan> correctly, setting flag_pic is actually a bug-fix when compiling
>Alan> powerpc64-linux shared libs.  We have the standard ELF binding of
>Alan> global syms.  That is, global functions may be overridden by functions
>Alan> in another shared library or by the main application.
>
>Alan> So powerpc64-linux-gcc should allow -fpic/PIC to twiddle flag_pic for
>Alan> binds_local_p, and users should set -fPIC when compiling shared libs
>Alan> as is common on other ELF targets.  We could use another flag, because
>Alan> like that annoying rs6000.c warning says "all code is position
>Alan> independent" on ppc64, but that would make powerpc64-linux just that
>Alan> more odd.  Lots of packages set -fPIC to mean "compile me code for a
>Alan> shared library".
>
>         This is what the patch that I applied to both gcc-3.2 and the
>trunk already does, without utilizing the generic infrastructure.  The
>PowerPC port currently does not use the targetm.binds_local_p.  We can
>discuss evolving to the generic infrastructure for GCC 3.4.

What might be acceptable for 3.3 still is the appended patch which 
basically just copies the varasm routines to rs6000.c and adds SDATA2 and 
AIXELF PIC handling. Briefly tested on powerpc-linux-gnu without 
regressions. Can the AIXELF people give it a try? Sorry, no changelog yet, 
I'm in a hurry right now.

Franz.



[-- Attachment #2: gcc-ppcsections-1.patch --]
[-- Type: application/octet-stream, Size: 15128 bytes --]

Index: gcc/config/rs6000/rs6000.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/rs6000/rs6000.c,v
retrieving revision 1.368
diff -u -p -r1.368 rs6000.c
--- gcc/config/rs6000/rs6000.c	23 Aug 2002 18:02:22 -0000	1.368
+++ gcc/config/rs6000/rs6000.c	30 Aug 2002 18:08:39 -0000
@@ -196,9 +196,11 @@ static unsigned int rs6000_elf_section_t
 							   int));
 static void rs6000_elf_asm_out_constructor PARAMS ((rtx, int));
 static void rs6000_elf_asm_out_destructor PARAMS ((rtx, int));
-static void rs6000_elf_select_section PARAMS ((tree, int,
-						 unsigned HOST_WIDE_INT));
-static void rs6000_elf_unique_section PARAMS ((tree, int));
+static bool rs6000_in_small_data_p PARAMS ((tree));
+
+void rs6000_elf_select_section PARAMS ((tree, int,
+					unsigned HOST_WIDE_INT));
+void rs6000_elf_unique_section PARAMS ((tree, int));
 static void rs6000_elf_select_rtx_section PARAMS ((enum machine_mode, rtx,
 						   unsigned HOST_WIDE_INT));
 static void rs6000_elf_encode_section_info PARAMS ((tree, int));
@@ -308,7 +310,8 @@ static const char alt_reg_names[][8] =
 #define TARGET_ATTRIBUTE_TABLE rs6000_attribute_table
 #undef TARGET_SET_DEFAULT_TYPE_ATTRIBUTES
 #define TARGET_SET_DEFAULT_TYPE_ATTRIBUTES rs6000_set_default_type_attributes
-
+#undef TARGET_IN_SMALL_DATA_P
+#define TARGET_IN_SMALL_DATA_P rs6000_in_small_data_p
 #undef TARGET_ASM_ALIGNED_DI_OP
 #define TARGET_ASM_ALIGNED_DI_OP DOUBLE_INT_ASM_OP
 
@@ -6503,7 +6506,7 @@ mtcrf_operation (op, mode)
       maskval = 1 << (MAX_CR_REGNO - REGNO (SET_DEST (exp)));
       
       if (GET_CODE (unspec) != UNSPEC
-	  || XINT (unspec, 1) != 20
+	  || XINT (unspec, 1) != UNSPEC_MOVESI_TO_CR
 	  || XVECLEN (unspec, 0) != 2
 	  || XVECEXP (unspec, 0, 0) != src_reg
 	  || GET_CODE (XVECEXP (unspec, 0, 1)) != CONST_INT
@@ -9633,7 +9636,7 @@ get_TOC_alias_set ()
 }   
 
 /* This retuns nonzero if the current function uses the TOC.  This is
-   determined by the presence of (unspec ... 7), which is generated by
+   determined by the presence of (unspec ... UNSPEC_TOCADR), which is generated by
    the various load_toc_* patterns.  */
 
 int
@@ -9650,7 +9653,7 @@ uses_TOC () 
 	  if (GET_CODE (pat) == PARALLEL) 
 	    for (i = 0; i < XVECLEN (PATTERN (insn), 0); i++)
 	      if (GET_CODE (XVECEXP (PATTERN (insn), 0, i)) == UNSPEC 
-		 && XINT (XVECEXP (PATTERN (insn), 0, i), 1) == 7)
+		 && XINT (XVECEXP (PATTERN (insn), 0, i), 1) == UNSPEC_TOCADR)
 		  return 1;
 	}
     return 0;
@@ -10845,7 +10848,7 @@ rs6000_emit_epilogue (sibcall)
 		RTVEC_ELT (r, 1) = GEN_INT (1 << (7-i));
 		RTVEC_ELT (p, ndx) =
 		  gen_rtx_SET (VOIDmode, gen_rtx_REG (CCmode, CR0_REGNO+i), 
-			       gen_rtx_UNSPEC (CCmode, r, 20));
+			       gen_rtx_UNSPEC (CCmode, r, UNSPEC_MOVESI_TO_CR));
 		ndx++;
 	      }
 	  emit_insn (gen_rtx_PARALLEL (VOIDmode, p));
@@ -12416,137 +12419,293 @@ rs6000_elf_select_rtx_section (mode, x, 
     default_elf_select_rtx_section (mode, x, align);
 }
 
-/* A C statement or statements to switch to the appropriate
-   section for output of DECL.  DECL is either a `VAR_DECL' node
-   or a constant of some sort.  RELOC indicates whether forming
-   the initial value of DECL requires link-time relocations.  */
 
-static void
-rs6000_elf_select_section (decl, reloc, align)
+static bool
+rs6000_in_small_data_p (decl)
      tree decl;
-     int reloc;
-     unsigned HOST_WIDE_INT align ATTRIBUTE_UNUSED;
 {
-  int size = int_size_in_bytes (TREE_TYPE (decl));
-  bool needs_sdata;
-  bool readonly;
-  static void (* const sec_funcs[4]) PARAMS ((void)) = {
-    &readonly_data_section,
-    &sdata2_section,
-    &data_section,
-    &sdata_section
-  };
+  /* We want to merge strings, so we never consider them small data.  */
+  if (TREE_CODE (decl) == STRING_CST)
+    return false;
   
-  needs_sdata = (size > 0 
-		 && size <= g_switch_value
-		 && rs6000_sdata != SDATA_NONE
-		 && (rs6000_sdata != SDATA_DATA || TREE_PUBLIC (decl)));
+  if (TREE_CODE (decl) == VAR_DECL && DECL_SECTION_NAME (decl))
+    {
+      const char *section = TREE_STRING_POINTER (DECL_SECTION_NAME (decl));
+      if (strcmp (section, ".sdata") == 0
+	  || strcmp (section, ".sdata2") == 0
+	  || strcmp (section, ".sbss") == 0)
+	return true;
+    }
+  else
+    {
+      int size = int_size_in_bytes (TREE_TYPE (decl));
+      return (size > 0
+	      && size <= g_switch_value
+	      && rs6000_sdata != SDATA_NONE
+	      && (rs6000_sdata != SDATA_DATA || TREE_PUBLIC (decl)));
+    }
+}
 
-  if (TREE_CODE (decl) == STRING_CST)
-    readonly = !flag_writable_strings;
+/* A helper function for default_elf_select_section and
+   default_elf_unique_section.  Categorizes the DECL.  */
+
+enum rs6000_section_category
+{
+  SECCAT_TEXT,
+
+  SECCAT_RODATA,
+  SECCAT_RODATA_MERGE_STR,
+  SECCAT_RODATA_MERGE_STR_INIT,
+  SECCAT_RODATA_MERGE_CONST,
+
+  SECCAT_DATA,
+
+  /* To optimize loading of shared programs, define following subsections
+     of data section:
+        _REL    Contains data that has relocations, so they get grouped
+                together and dynamic linker will visit fewer pages in memory.
+        _RO     Contains data that is otherwise read-only.  This is useful
+                with prelinking as most relocations won't be dynamically
+                linked and thus stay read only.
+        _LOCAL  Marks data containing relocations only to local objects.
+                These relocations will get fully resolved by prelinking.  */
+  SECCAT_DATA_REL,
+  SECCAT_DATA_REL_LOCAL,
+  SECCAT_DATA_REL_RO,
+  SECCAT_DATA_REL_RO_LOCAL,
+
+  SECCAT_SDATA,
+  SECCAT_SDATA2,
+  SECCAT_TDATA,
+
+  SECCAT_BSS,
+  SECCAT_SBSS,
+  SECCAT_TBSS
+};
+
+static enum rs6000_section_category rs6000_categorize_decl_for_section PARAMS ((tree, int, int));
+
+static enum rs6000_section_category
+rs6000_categorize_decl_for_section (decl, reloc, pic)
+     tree decl;
+     int reloc;
+     int pic;
+{
+  enum rs6000_section_category ret;
+
+  if (TREE_CODE (decl) == FUNCTION_DECL)
+    return SECCAT_TEXT;
+  else if (TREE_CODE (decl) == STRING_CST)
+    {
+      if (flag_writable_strings)
+        return SECCAT_DATA;
+      else
+        return SECCAT_RODATA_MERGE_STR;
+    }
   else if (TREE_CODE (decl) == VAR_DECL)
-    readonly = (!((flag_pic || DEFAULT_ABI == ABI_AIX) && reloc)
-		&& TREE_READONLY (decl)
-		&& !TREE_SIDE_EFFECTS (decl)
-		&& DECL_INITIAL (decl)
-		&& DECL_INITIAL (decl) != error_mark_node
-		&& TREE_CONSTANT (DECL_INITIAL (decl)));
+    {
+      if (DECL_INITIAL (decl) == NULL
+          || DECL_INITIAL (decl) == error_mark_node)
+        ret = SECCAT_BSS;
+      else if (! TREE_READONLY (decl)
+               || TREE_SIDE_EFFECTS (decl)
+               || ! TREE_CONSTANT (DECL_INITIAL (decl)))
+        {
+          if (pic && (reloc & 2))
+            ret = SECCAT_DATA_REL;
+          else if (pic && reloc)
+            ret = SECCAT_DATA_REL_LOCAL;
+          else
+            ret = SECCAT_DATA;
+        }
+      else if (pic && (reloc & 2))
+        ret = SECCAT_DATA_REL_RO;
+      else if (pic && reloc)
+        ret = SECCAT_DATA_REL_RO_LOCAL;
+      else if (flag_merge_constants < 2)
+        /* C and C++ don't allow different variables to share the same
+           location.  -fmerge-all-constants allows even that (at the
+           expense of not conforming).  */
+        ret = SECCAT_RODATA;
+      else if (TREE_CODE (DECL_INITIAL (decl)) == STRING_CST)
+        ret = SECCAT_RODATA_MERGE_STR_INIT;
+      else
+        ret = SECCAT_RODATA_MERGE_CONST;
+    }
   else if (TREE_CODE (decl) == CONSTRUCTOR)
-    readonly = (!((flag_pic || DEFAULT_ABI == ABI_AIX) && reloc)
-		&& !TREE_SIDE_EFFECTS (decl)
-		&& TREE_CONSTANT (decl));
+    {
+      if ((pic && reloc)
+          || TREE_SIDE_EFFECTS (decl)
+          || ! TREE_CONSTANT (decl))
+        ret = SECCAT_DATA;
+      else
+        ret = SECCAT_RODATA;
+    }
   else
-    readonly = !((flag_pic || DEFAULT_ABI == ABI_AIX) && reloc);
+    ret = SECCAT_RODATA;
 
-  if (needs_sdata && rs6000_sdata != SDATA_EABI)
-    readonly = false;
-  
-  (*sec_funcs[(readonly ? 0 : 2) + (needs_sdata ? 1 : 0)])();
+  /* There are no read-only thread-local sections.  */
+  if (TREE_CODE (decl) == VAR_DECL && DECL_THREAD_LOCAL (decl))
+    {
+      if (ret == SECCAT_BSS)
+        ret = SECCAT_TBSS;
+      else
+        ret = SECCAT_TDATA;
+    }
+
+  /* If the target uses small data sections, select it.  */
+  else if ((*targetm.in_small_data_p) (decl))
+    {
+      if (ret == SECCAT_BSS)
+        ret = SECCAT_SBSS;
+      else
+        ret = SECCAT_SDATA;
+    }
+
+  /* With EABI we put small readonly data into .sdata2.  */
+  if (ret == SECCAT_RODATA && rs6000_sdata == SDATA_EABI
+      && (*targetm.in_small_data_p) (decl))
+    ret = SECCAT_SDATA2;
+
+  return ret;
 }
 
-/* A C statement to build up a unique section name, expressed as a
-   STRING_CST node, and assign it to DECL_SECTION_NAME (decl).
-   RELOC indicates whether the initial value of EXP requires
-   link-time relocations.  If you do not define this macro, GCC will use
-   the symbol name prefixed by `.' as the section name.  Note - this
-   macro can now be called for uninitialized data items as well as
-   initialised data and functions.  */
+/* Select a section based on the above categorization.  */
 
-static void
+void
+rs6000_elf_select_section (decl, reloc, align)
+     tree decl;
+     int reloc;
+     unsigned HOST_WIDE_INT align;
+{
+  bool pic = flag_pic || DEFAULT_ABI == ABI_AIX;
+
+  switch (rs6000_categorize_decl_for_section (decl, reloc, pic))
+    {
+    case SECCAT_TEXT:
+      /* We're not supposed to be called on FUNCTION_DECLs.  */
+      abort ();
+    case SECCAT_RODATA:
+      readonly_data_section ();
+      break;
+    case SECCAT_RODATA_MERGE_STR:
+      mergeable_string_section (decl, align, 0);
+      break;
+    case SECCAT_RODATA_MERGE_STR_INIT:
+      mergeable_string_section (DECL_INITIAL (decl), align, 0);
+      break;
+    case SECCAT_RODATA_MERGE_CONST:
+      mergeable_constant_section (DECL_MODE (decl), align, 0);
+      break;
+    case SECCAT_DATA:
+      data_section ();
+      break;
+    case SECCAT_DATA_REL:
+      named_section (NULL_TREE, ".data.rel", reloc);
+      break;
+    case SECCAT_DATA_REL_LOCAL:
+      named_section (NULL_TREE, ".data.rel.local", reloc);
+      break;
+    case SECCAT_DATA_REL_RO:
+      named_section (NULL_TREE, ".data.rel.ro", reloc);
+      break;
+    case SECCAT_DATA_REL_RO_LOCAL:
+      named_section (NULL_TREE, ".data.rel.ro.local", reloc);
+      break;
+    case SECCAT_SDATA:
+      named_section (NULL_TREE, ".sdata", reloc);
+      break;
+    case SECCAT_SDATA2:
+      named_section (NULL_TREE, ".sdata2", reloc);
+      break;
+    case SECCAT_TDATA:
+      named_section (NULL_TREE, ".tdata", reloc);
+      break;
+    case SECCAT_BSS:
+#ifdef BSS_SECTION_ASM_OP
+      bss_section ();
+#else
+      named_section (NULL_TREE, ".bss", reloc);
+#endif
+      break;
+    case SECCAT_SBSS:
+      named_section (NULL_TREE, ".sbss", reloc);
+      break;
+    case SECCAT_TBSS:
+      named_section (NULL_TREE, ".tbss", reloc);
+      break;
+    default:
+      abort ();
+    }
+}
+
+/* Construct a unique section name based on the decl name and the
+   categorization performed above.  */
+
+void
 rs6000_elf_unique_section (decl, reloc)
      tree decl;
      int reloc;
 {
-  int len;
-  int sec;
-  const char *name;
+  bool one_only = DECL_ONE_ONLY (decl);
+  bool pic = flag_pic || DEFAULT_ABI == ABI_AIX;
+  const char *prefix, *name;
+  size_t nlen, plen;
   char *string;
-  const char *prefix;
 
-  static const char *const prefixes[7][2] =
-  {
-    { ".rodata.", ".gnu.linkonce.r." },
-    { ".sdata2.", ".gnu.linkonce.s2." },
-    { ".data.",   ".gnu.linkonce.d." },
-    { ".sdata.",  ".gnu.linkonce.s." },
-    { ".bss.",    ".gnu.linkonce.b." },
-    { ".sbss.",   ".gnu.linkonce.sb." },
-    { ".text.",   ".gnu.linkonce.t." }
-  };
-
-  if (TREE_CODE (decl) == FUNCTION_DECL)
-    sec = 6;
-  else
+  switch (rs6000_categorize_decl_for_section (decl, reloc, pic))
     {
-      bool readonly;
-      bool needs_sdata;
-      int size;
-
-      if (TREE_CODE (decl) == STRING_CST)
-	readonly = !flag_writable_strings;
-      else if (TREE_CODE (decl) == VAR_DECL)
-	readonly = (!((flag_pic || DEFAULT_ABI == ABI_AIX) && reloc)
-		    && TREE_READONLY (decl)
-		    && !TREE_SIDE_EFFECTS (decl)
-		    && TREE_CONSTANT (DECL_INITIAL (decl)));
-      else
-	readonly = !((flag_pic || DEFAULT_ABI == ABI_AIX) && reloc);
-
-      size = int_size_in_bytes (TREE_TYPE (decl));
-      needs_sdata = (size > 0 
-		     && size <= g_switch_value
-		     && rs6000_sdata != SDATA_NONE
-		     && (rs6000_sdata != SDATA_DATA || TREE_PUBLIC (decl)));
-
-      if (DECL_INITIAL (decl) == NULL
-	  || DECL_INITIAL (decl) == error_mark_node)
-	sec = 4;
-      else if (!readonly)
-	sec = 2;
-      else
-	sec = 0;
-
-      if (needs_sdata)
-	{
-	  /* .sdata2 is only for EABI.  */
-	  if (sec == 0 && rs6000_sdata != SDATA_EABI)
-	    sec = 2;
-	  sec += 1;
-	}
+    case SECCAT_TEXT:
+      prefix = one_only ? ".gnu.linkonce.t." : ".text.";
+      break;
+    case SECCAT_RODATA:
+    case SECCAT_RODATA_MERGE_STR:
+    case SECCAT_RODATA_MERGE_STR_INIT:
+    case SECCAT_RODATA_MERGE_CONST:
+      prefix = one_only ? ".gnu.linkonce.r." : ".rodata.";
+      break;
+    case SECCAT_DATA:
+    case SECCAT_DATA_REL:
+    case SECCAT_DATA_REL_LOCAL:
+    case SECCAT_DATA_REL_RO:
+    case SECCAT_DATA_REL_RO_LOCAL:
+      prefix = one_only ? ".gnu.linkonce.d." : ".data.";
+      break;
+    case SECCAT_SDATA:
+      prefix = one_only ? ".gnu.linkonce.s." : ".sdata.";
+      break;
+    case SECCAT_SDATA2:
+      prefix = one_only ? ".gnu.linkonce.s2." : ".sdata2.";
+      break;
+    case SECCAT_BSS:
+      prefix = one_only ? ".gnu.linkonce.b." : ".bss.";
+      break;
+    case SECCAT_SBSS:
+      prefix = one_only ? ".gnu.linkonce.sb." : ".sbss.";
+      break;
+    case SECCAT_TDATA:
+      prefix = one_only ? ".gnu.linkonce.td." : ".tdata.";
+      break;
+    case SECCAT_TBSS:
+      prefix = one_only ? ".gnu.linkonce.tb." : ".tbss.";
+      break;
+    default:
+      abort ();
     }
+  plen = strlen (prefix);
 
   name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
-  name = (*targetm.strip_name_encoding) (name);
-  prefix = prefixes[sec][DECL_ONE_ONLY (decl)];
-  len    = strlen (name) + strlen (prefix);
-  string = alloca (len + 1);
-  
-  sprintf (string, "%s%s", prefix, name);
-  
-  DECL_SECTION_NAME (decl) = build_string (len, string);
+  name = (* targetm.strip_name_encoding) (name);
+  nlen = strlen (name);
+
+  string = alloca (nlen + plen + 1);
+  memcpy (string, prefix, plen);
+  memcpy (string + plen, name, nlen + 1);
+
+  DECL_SECTION_NAME (decl) = build_string (nlen + plen, string);
 }
 
-\f
+
 /* If we are referencing a function that is static or is known to be
    in this file, make the SYMBOL_REF special.  We can use this to indicate
    that we can branch to this function without emitting a no-op after the

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Alan Modra writes:

Alan> Uh, oh.  If I understand binds_local_p and mark_constant_function
Alan> correctly, setting flag_pic is actually a bug-fix when compiling
Alan> powerpc64-linux shared libs.  We have the standard ELF binding of
Alan> global syms.  That is, global functions may be overridden by functions
Alan> in another shared library or by the main application.

Alan> So powerpc64-linux-gcc should allow -fpic/PIC to twiddle flag_pic for
Alan> binds_local_p, and users should set -fPIC when compiling shared libs
Alan> as is common on other ELF targets.  We could use another flag, because
Alan> like that annoying rs6000.c warning says "all code is position
Alan> independent" on ppc64, but that would make powerpc64-linux just that
Alan> more odd.  Lots of packages set -fPIC to mean "compile me code for a
Alan> shared library".

	This is what the patch that I applied to both gcc-3.2 and the
trunk already does, without utilizing the generic infrastructure.  The
PowerPC port currently does not use the targetm.binds_local_p.  We can
discuss evolving to the generic infrastructure for GCC 3.4.

David

Re: [RFC] PowerPC select_section / unique_section

[Alan Modra]

On Fri, Aug 30, 2002 at 11:00:23PM +0930, Alan Modra wrote:
> So powerpc64-linux-gcc should allow -fpic/PIC to twiddle flag_pic for
> binds_local_p

Just a note to save any duplication of effort:
I have a patch that does this and makes rs6000/* effectively ignore
flag_pic as regarding code generation when ABI_AIX.  I'll post it in
the morning if my overnight builds are successful.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] PowerPC select_section / unique_section

[Alan Modra]

On Thu, Aug 29, 2002 at 06:01:18PM -0700, Richard Henderson wrote:
> On Thu, Aug 22, 2002 at 11:29:31AM +0930, Alan Modra wrote:
> > Ouch.  Forget my suggestion re decl_readonly_section.  Hmm, would
> > you accept a patch that cleans up flag_pic in rs6000.c?  ie. has
> > flag_pic always set for ABI_AIX?
> 
> That will pessimize ppc64-elf name binding.  (I.e. binds_local_p.)

Uh, oh.  If I understand binds_local_p and mark_constant_function
correctly, setting flag_pic is actually a bug-fix when compiling
powerpc64-linux shared libs.  We have the standard ELF binding of
global syms.  That is, global functions may be overridden by functions
in another shared library or by the main application.

So powerpc64-linux-gcc should allow -fpic/PIC to twiddle flag_pic for
binds_local_p, and users should set -fPIC when compiling shared libs
as is common on other ELF targets.  We could use another flag, because
like that annoying rs6000.c warning says "all code is position
independent" on ppc64, but that would make powerpc64-linux just that
more odd.  Lots of packages set -fPIC to mean "compile me code for a
shared library".

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] PowerPC select_section / unique_section

[Richard Henderson]

On Wed, Aug 21, 2002 at 10:25:26PM -0400, David Edelsohn wrote:
> Alan> Setting flag_pic to zero when
> Alan> we are PIC amounts to lying to the rest of the compiler.
> 
> 	Not really.  GCC handled PIC just fine before flag_pic was
> introduced.  flag_pic really means flag_got.  It really means enable a
> particular way of implementing PIC support, not enable PIC codegen.  It
> would be nice if flag_pic meant PIC, but the meaning is confused with the
> implementation. 

Indeed.  For ELF what it actually means is "compiling for
inclusion in a shared library".  Existing usage of the
command-line switch is way too entrenched to rename it.


r~

Re: [RFC] PowerPC select_section / unique_section

[Richard Henderson]

On Thu, Aug 22, 2002 at 11:29:31AM +0930, Alan Modra wrote:
> Ouch.  Forget my suggestion re decl_readonly_section.  Hmm, would
> you accept a patch that cleans up flag_pic in rs6000.c?  ie. has
> flag_pic always set for ABI_AIX?

That will pessimize ppc64-elf name binding.  (I.e. binds_local_p.)


r~

Re: [RFC] PowerPC select_section / unique_section

[Richard Henderson]

On Wed, Aug 21, 2002 at 03:01:20PM -0400, David Edelsohn wrote:
> Yes, I considered setting and unsetting flag_pic, but that's ugly.

Nevertheless, see ia64_aix_unique_section and ia64_aix_select_rtx_section
in which exactly this is done.  Personally, I think this is the best
solution.


r~

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Alan Modra writes:

Alan> Setting flag_pic to zero when
Alan> we are PIC amounts to lying to the rest of the compiler.

	Not really.  GCC handled PIC just fine before flag_pic was
introduced.  flag_pic really means flag_got.  It really means enable a
particular way of implementing PIC support, not enable PIC codegen.  It
would be nice if flag_pic meant PIC, but the meaning is confused with the
implementation. 

David

Re: [RFC] PowerPC select_section / unique_section

[Alan Modra]

On Wed, Aug 21, 2002 at 10:07:13PM -0400, David Edelsohn wrote:
> >>>>> Alan Modra writes:
> 
> Alan> Ouch.  Forget my suggestion re decl_readonly_section.  Hmm, would
> Alan> you accept a patch that cleans up flag_pic in rs6000.c?  ie. has
> Alan> flag_pic always set for ABI_AIX?
> 
> 	Have you actually tried that?

Not yet, but I think it's worth doing.  Setting flag_pic to zero when
we are PIC amounts to lying to the rest of the compiler.

>  flag_pic interferes with ABI_AIX
> TOC register usage.  ABI_AIX does not use the flag_pic machinery.

Yes, I know.  I'm talking about adding a DEFAULT_ABI test with all
uses of flag_pic in rs6000/*.  Many already do so.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Alan Modra writes:

Alan> Ouch.  Forget my suggestion re decl_readonly_section.  Hmm, would
Alan> you accept a patch that cleans up flag_pic in rs6000.c?  ie. has
Alan> flag_pic always set for ABI_AIX?

	Have you actually tried that?  flag_pic interferes with ABI_AIX
TOC register usage.  ABI_AIX does not use the flag_pic machinery.

	Now that I have fixed the ASM_GLOBALIZE_LABEL breakage, I am going
to get back to testing the select_section patch for GCC 3.2 and GCC 3.3.
We can look at cleaning up select_section and unique_section for GCC 3.4,
but it's too late for GCC 3.2 and GCC 3.3.  I only am modifying the
current form of those functions enough to fix bugs.

David

Re: [RFC] PowerPC select_section / unique_section

[Alan Modra]

On Wed, Aug 21, 2002 at 02:25:06PM -0400, David Edelsohn wrote:
> 	PowerPC cannot use the default functions in varasm.c because of
> its assumptions (e.g., flag_pic).

Ouch.  Forget my suggestion re decl_readonly_section.  Hmm, would
you accept a patch that cleans up flag_pic in rs6000.c?  ie. has
flag_pic always set for ABI_AIX?

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Alan Modra writes:

Alan> Why not use decl_readonly_section?
Alan> Similarly for the other functions.  I've used this on gcc-3.2 based
Alan> powerpc64-linux for a couple of weeks.  Patch available on request.

	As I replied to Franz, testing flag_pic is not sufficient for
PPC64.  Any patch using the varasm.c default functions without further
changes is wrong.

David

Re: [RFC] PowerPC select_section / unique_section

[Alan Modra]

On Wed, Aug 21, 2002 at 01:20:18PM -0400, David Edelsohn wrote:
> 	* config/rs6000/rs6000.c (rs6000_elf_select_section): Treat
> 	DEFAULT_ABI == ABI_AIX like PIC.  Test PIC & reloc for readonly
> 	default.
> 	(rs6000_elf_unique_section): Treat DEFAULT_ABI == ABI_AIX like
> 	PIC.
> 	(rs6000_xcoff_select_section): Update to recent readonly
> 	algorithm.

Why not use decl_readonly_section?  rs6000_elf_select_section ends up
looking like:

  needs_sdata = (size > 0 
		 && size <= g_switch_value
		 && rs6000_sdata != SDATA_NONE
		 && (rs6000_sdata != SDATA_DATA || TREE_PUBLIC (decl)));

  readonly = 0;
  if (!needs_sdata || rs6000_sdata == SDATA_EABI)
    readonly = decl_readonly_section (decl, reloc);
  
  (*sec_funcs[(readonly ? 0 : 2) + (needs_sdata ? 1 : 0)])();

Similarly for the other functions.  I've used this on gcc-3.2 based
powerpc64-linux for a couple of weeks.  Patch available on request.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Franz Sirl writes:

Franz> Well, we could save flag_pic around calling these functions or change the 
Franz> flag_pic checks in varasm.c to a target hook.

	Yes, I considered setting and unsetting flag_pic, but that's
ugly.  The only way I see to use the default functions is to add a "pic"
parameter to the functions instead of using the GCC flag_pic global.

David

Re: [RFC] PowerPC select_section / unique_section

[Franz Sirl]

On Mittwoch, 21. August 2002 20:25, David Edelsohn wrote:
> >>>>> Franz Sirl writes:
>
> Franz> I suggested to unify the handling and the followup discussion on
> Franz> how to better re-use the stuff in varasm.c. I haven't come around to
> Franz> implement this yet unfortunately (and it doesn't look like I will be
> able to Franz> tackle this anytime soon).
>
> 	PowerPC cannot use the default functions in varasm.c because of
> its assumptions (e.g., flag_pic).  I already investigated that and I do
> remember the earlier discussion.

Well, we could save flag_pic around calling these functions or change the 
flag_pic checks in varasm.c to a target hook.

> 	My patch is not a cleanup, it's a correctness issue.

I understand that, but I consider reusing as much as possible of the varasm 
functions a longterm correctness issue.

Franz.

Re: [RFC] PowerPC select_section / unique_section

[David Edelsohn]

>>>>> Franz Sirl writes:

Franz> I suggested to unify the handling and the followup discussion on 
Franz> how to better re-use the stuff in varasm.c. I haven't come around to 
Franz> implement this yet unfortunately (and it doesn't look like I will be able to 
Franz> tackle this anytime soon).

	PowerPC cannot use the default functions in varasm.c because of
its assumptions (e.g., flag_pic).  I already investigated that and I do
remember the earlier discussion.

	My patch is not a cleanup, it's a correctness issue.

David

Re: [RFC] PowerPC select_section / unique_section

[Franz Sirl]

On Mittwoch, 21. August 2002 19:20, David Edelsohn wrote:
> While investigating a problem with PowerPC ELF section decisions,
> I realized that the functions need to treat PPC64 as if flag_pic were set.
> I also noticed a discrepancy between the PowerPC definition and the
> default definition in varasm.c: rs6000_elf_select_section should not
> default to readonly when reloc is defined.
>
> 	My remaining concern is that the readonly algorithm in
> rs6000_elf_unique_section does not match the algorithm in
> rs6000_elf_select_section: the handling of decl CONSTRUCTOR and the
> handling of default readonly.  Should the definitions match?

 See

	<http://gcc.gnu.org/ml/gcc-patches/2002-05/subjects.html#01551>

for a patch I suggested to unify the handling and the followup discussion on 
how to better re-use the stuff in varasm.c. I haven't come around to 
implement this yet unfortunately (and it doesn't look like I will be able to 
tackle this anytime soon).

Franz.

[RFC] PowerPC select_section / unique_section

[David Edelsohn]

	While investigating a problem with PowerPC ELF section decisions,
I realized that the functions need to treat PPC64 as if flag_pic were set.
I also noticed a discrepancy between the PowerPC definition and the
default definition in varasm.c: rs6000_elf_select_section should not
default to readonly when reloc is defined.

	My remaining concern is that the readonly algorithm in
rs6000_elf_unique_section does not match the algorithm in
rs6000_elf_select_section: the handling of decl CONSTRUCTOR and the
handling of default readonly.  Should the definitions match?

Thanks, David


	* config/rs6000/rs6000.c (rs6000_elf_select_section): Treat
	DEFAULT_ABI == ABI_AIX like PIC.  Test PIC & reloc for readonly
	default.
	(rs6000_elf_unique_section): Treat DEFAULT_ABI == ABI_AIX like
	PIC.
	(rs6000_xcoff_select_section): Update to recent readonly
	algorithm.

Index: rs6000.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/rs6000/rs6000.c,v
retrieving revision 1.364
diff -c -p -r1.364 rs6000.c
*** rs6000.c	19 Aug 2002 16:32:53 -0000	1.364
--- rs6000.c	21 Aug 2002 16:42:25 -0000
*************** rs6000_elf_select_section (decl, reloc, 
*** 12432,12439 ****
       unsigned HOST_WIDE_INT align ATTRIBUTE_UNUSED;
  {
    int size = int_size_in_bytes (TREE_TYPE (decl));
!   int needs_sdata;
!   int readonly;
    static void (* const sec_funcs[4]) PARAMS ((void)) = {
      &readonly_data_section,
      &sdata2_section,
--- 12462,12469 ----
       unsigned HOST_WIDE_INT align ATTRIBUTE_UNUSED;
  {
    int size = int_size_in_bytes (TREE_TYPE (decl));
!   bool needs_sdata;
!   bool readonly;
    static void (* const sec_funcs[4]) PARAMS ((void)) = {
      &readonly_data_section,
      &sdata2_section,
*************** rs6000_elf_select_section (decl, reloc, 
*** 12447,12468 ****
  		 && (rs6000_sdata != SDATA_DATA || TREE_PUBLIC (decl)));
  
    if (TREE_CODE (decl) == STRING_CST)
!     readonly = ! flag_writable_strings;
    else if (TREE_CODE (decl) == VAR_DECL)
!     readonly = (! (flag_pic && reloc)
  		&& TREE_READONLY (decl)
! 		&& ! TREE_SIDE_EFFECTS (decl)
  		&& DECL_INITIAL (decl)
  		&& DECL_INITIAL (decl) != error_mark_node
  		&& TREE_CONSTANT (DECL_INITIAL (decl)));
    else if (TREE_CODE (decl) == CONSTRUCTOR)
!     readonly = (! (flag_pic && reloc)
! 		&& ! TREE_SIDE_EFFECTS (decl)
  		&& TREE_CONSTANT (decl));
    else
!     readonly = 1;
    if (needs_sdata && rs6000_sdata != SDATA_EABI)
!     readonly = 0;
    
    (*sec_funcs[(readonly ? 0 : 2) + (needs_sdata ? 1 : 0)])();
  }
--- 12477,12499 ----
  		 && (rs6000_sdata != SDATA_DATA || TREE_PUBLIC (decl)));
  
    if (TREE_CODE (decl) == STRING_CST)
!     readonly = !flag_writable_strings;
    else if (TREE_CODE (decl) == VAR_DECL)
!     readonly = (!((flag_pic || DEFAULT_ABI == ABI_AIX) && reloc)
  		&& TREE_READONLY (decl)
! 		&& !TREE_SIDE_EFFECTS (decl)
  		&& DECL_INITIAL (decl)
  		&& DECL_INITIAL (decl) != error_mark_node
  		&& TREE_CONSTANT (DECL_INITIAL (decl)));
    else if (TREE_CODE (decl) == CONSTRUCTOR)
!     readonly = (!((flag_pic || DEFAULT_ABI == ABI_AIX) && reloc)
! 		&& !TREE_SIDE_EFFECTS (decl)
  		&& TREE_CONSTANT (decl));
    else
!     readonly = !((flag_pic || DEFAULT_ABI == ABI_AIX) && reloc);
! 
    if (needs_sdata && rs6000_sdata != SDATA_EABI)
!     readonly = false;
    
    (*sec_funcs[(readonly ? 0 : 2) + (needs_sdata ? 1 : 0)])();
  }
*************** rs6000_elf_unique_section (decl, reloc)
*** 12501,12517 ****
      sec = 6;
    else
      {
!       int readonly;
!       int needs_sdata;
        int size;
  
-       readonly = 1;
        if (TREE_CODE (decl) == STRING_CST)
! 	readonly = ! flag_writable_strings;
        else if (TREE_CODE (decl) == VAR_DECL)
! 	readonly = (! (flag_pic && reloc)
  		    && TREE_READONLY (decl)
! 		    && ! TREE_SIDE_EFFECTS (decl)
  		    && TREE_CONSTANT (DECL_INITIAL (decl)));
  
        size = int_size_in_bytes (TREE_TYPE (decl));
--- 12532,12547 ----
      sec = 6;
    else
      {
!       bool readonly = true;
!       bool needs_sdata;
        int size;
  
        if (TREE_CODE (decl) == STRING_CST)
! 	readonly = !flag_writable_strings;
        else if (TREE_CODE (decl) == VAR_DECL)
! 	readonly = (!((flag_pic || DEFAULT_ABI == ABI_AIX) && reloc)
  		    && TREE_READONLY (decl)
! 		    && !TREE_SIDE_EFFECTS (decl)
  		    && TREE_CONSTANT (DECL_INITIAL (decl)));
  
        size = int_size_in_bytes (TREE_TYPE (decl));
*************** rs6000_elf_unique_section (decl, reloc)
*** 12523,12529 ****
        if (DECL_INITIAL (decl) == 0
  	  || DECL_INITIAL (decl) == error_mark_node)
  	sec = 4;
!       else if (! readonly)
  	sec = 2;
        else
  	sec = 0;
--- 12553,12559 ----
        if (DECL_INITIAL (decl) == 0
  	  || DECL_INITIAL (decl) == error_mark_node)
  	sec = 4;
!       else if (!readonly)
  	sec = 2;
        else
  	sec = 0;
*************** xcoff_asm_named_section (name, flags)
*** 13104,13131 ****
  }
  
  static void
! rs6000_xcoff_select_section (exp, reloc, align)
!      tree exp;
       int reloc;
       unsigned HOST_WIDE_INT align ATTRIBUTE_UNUSED;
  {
!   if ((TREE_CODE (exp) == STRING_CST
!        && ! flag_writable_strings)
!       || (TREE_CODE_CLASS (TREE_CODE (exp)) == 'd'
! 	  && TREE_READONLY (exp) && ! TREE_THIS_VOLATILE (exp)
! 	  && DECL_INITIAL (exp)
! 	  && (DECL_INITIAL (exp) == error_mark_node
! 	      || TREE_CONSTANT (DECL_INITIAL (exp)))
! 	  && ! (reloc)))
      {
!       if (TREE_PUBLIC (exp))
          read_only_data_section ();
        else
          read_only_private_data_section ();
      }
    else
      {
!       if (TREE_PUBLIC (exp))
          data_section ();
        else
          private_data_section ();
--- 13134,13172 ----
  }
  
  static void
! rs6000_xcoff_select_section (decl, reloc, align)
!      tree decl;
       int reloc;
       unsigned HOST_WIDE_INT align ATTRIBUTE_UNUSED;
  {
!   bool readonly = false;
! 
!   if (TREE_CODE (decl) == STRING_CST)
!     readonly = !flag_writable_strings;
!   else if (TREE_CODE (decl) == VAR_DECL)
!     readonly = (!reloc
! 		&& TREE_READONLY (decl)
! 		&& !TREE_SIDE_EFFECTS (decl)
! 		&& DECL_INITIAL (decl)
! 		&& DECL_INITIAL (decl) != error_mark_node
! 		&& TREE_CONSTANT (DECL_INITIAL (decl)));
!   else if (TREE_CODE (decl) == CONSTRUCTOR)
!     readonly = (!reloc
! 		&& !TREE_SIDE_EFFECTS (decl)
! 		&& TREE_CONSTANT (decl));
!   else
!     readonly = !reloc;
! 
!   if (readonly)
      {
!       if (TREE_PUBLIC (decl))
          read_only_data_section ();
        else
          read_only_private_data_section ();
      }
    else
      {
!       if (TREE_PUBLIC (decl))
          data_section ();
        else
          private_data_section ();