[PATCH] fix hardcoded rs6000-xcoff csects alignment

[PATCH] fix hardcoded rs6000-xcoff csects alignment

[Olivier Hainque]

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

Hello,

rs6000-xcoff ports use hardcoded alignment factors for control
sections in a number of places, intended to match what BIGGEST_ALIGNMENT
allows.

BIGGEST_ALIGNMENT was bumped to 16bytes some time ago and the csect
alignments remained untouched, typically:

   xcoff.h
   ...
   /* Output before writable data.
      Align entire section to BIGGEST_ALIGNMENT.  */
   #define DATA_SECTION_ASM_OP "\t.csect .data[RW],3"
                                                  ^^^ 8bytes align

Attached is a patch to fix this together with a simple Ada testcase
(runs fine with the patch, raise Program_Error otherwise).

The patch was bootstrapped and regtested on powerpc-ibm-aix5.3.0.0.

Thanks in advance,

Olivier


2007-06-18  Olivier Hainque  <hainque@adacore.com>

	* config/rs6000/xcoff.h (XCOFF_CSECT_DEFAULT_ALIGNMENT_STR): New
	internal macro. Default alignment factor for csect directives, chosen
	to match what BIGGEST_ALIGNMENT allows.
	(DATA_SECTION_ASM_OP): Use it.
	* config/rs6000/rs6000.c (rs6000_xcoff_output_readonly_section_asm_op):
	Use XCOFF_CSECT_DEFAULT_ALIGNMENT_STR.
	(rs6000_xcoff_output_readwrite_section_asm_op): Likewise.
	(rs6000_xcoff_output_toc_section_asm_op): Likewise.

	testsuite/
	* gnat.dg/test_oalign.adb: New test.
	* gnat.dg/oalign1.ads: Part of new test.
	* gnat.dg/oalign2.ads: Part of new test.










[-- Attachment #2: csects-align.dif --]
[-- Type: text/plain, Size: 4662 bytes --]

Index: config/rs6000/xcoff.h
===================================================================
*** config/rs6000/xcoff.h	(revision 125179)
--- config/rs6000/xcoff.h	(working copy)
***************
*** 61,66 ****
--- 61,70 ----
  
  #define MAX_OFILE_ALIGNMENT 32768
  
+ /* Default alignment factor for csect directives, chosen to honor
+    BIGGEST_ALIGNMENT.  */
+ #define XCOFF_CSECT_DEFAULT_ALIGNMENT_STR "4"
+ 
  /* Return nonzero if this entry is to be written into the constant
     pool in a special way.  We do so if this is a SYMBOL_REF, LABEL_REF
     or a CONST containing one of them.  If -mfp-in-toc (the default),
***************
*** 278,286 ****
  /* Output before instructions.  */
  #define TEXT_SECTION_ASM_OP "\t.csect .text[PR]"
  
! /* Output before writable data.
!    Align entire section to BIGGEST_ALIGNMENT.  */
! #define DATA_SECTION_ASM_OP "\t.csect .data[RW],3"
  
  /* Define to prevent DWARF2 unwind info in the data section rather
     than in the .eh_frame section.  We do this because the AIX linker
--- 282,291 ----
  /* Output before instructions.  */
  #define TEXT_SECTION_ASM_OP "\t.csect .text[PR]"
  
! /* Output before writable data.  */
! #define DATA_SECTION_ASM_OP \
!   "\t.csect .data[RW]," XCOFF_CSECT_DEFAULT_ALIGNMENT_STR
! 
  
  /* Define to prevent DWARF2 unwind info in the data section rather
     than in the .eh_frame section.  We do this because the AIX linker
Index: config/rs6000/rs6000.c
===================================================================
*** config/rs6000/rs6000.c	(revision 125179)
--- config/rs6000/rs6000.c	(working copy)
*************** rs6000_xcoff_asm_globalize_label (FILE *
*** 19613,19620 ****
  static void
  rs6000_xcoff_output_readonly_section_asm_op (const void *directive)
  {
!   fprintf (asm_out_file, "\t.csect %s[RO],3\n",
! 	   *(const char *const *) directive);
  }
  
  /* Likewise for read-write sections.  */
--- 19625,19633 ----
  static void
  rs6000_xcoff_output_readonly_section_asm_op (const void *directive)
  {
!   fprintf (asm_out_file, "\t.csect %s[RO],%s\n",
! 	   *(const char *const *) directive,
! 	   XCOFF_CSECT_DEFAULT_ALIGNMENT_STR);
  }
  
  /* Likewise for read-write sections.  */
*************** rs6000_xcoff_output_readonly_section_asm
*** 19622,19629 ****
  static void
  rs6000_xcoff_output_readwrite_section_asm_op (const void *directive)
  {
!   fprintf (asm_out_file, "\t.csect %s[RW],3\n",
! 	   *(const char *const *) directive);
  }
  
  /* A get_unnamed_section callback, used for switching to toc_section.  */
--- 19635,19643 ----
  static void
  rs6000_xcoff_output_readwrite_section_asm_op (const void *directive)
  {
!   fprintf (asm_out_file, "\t.csect %s[RW],%s\n",
! 	   *(const char *const *) directive,
! 	   XCOFF_CSECT_DEFAULT_ALIGNMENT_STR);
  }
  
  /* A get_unnamed_section callback, used for switching to toc_section.  */
*************** rs6000_xcoff_output_toc_section_asm_op (
*** 19643,19649 ****
  	  toc_initialized = 1;
  	}
        fprintf (asm_out_file, "\t.csect toc_table[RW]%s\n",
! 	       (TARGET_32BIT ? "" : ",3"));
      }
    else
      fputs ("\t.toc\n", asm_out_file);
--- 19657,19664 ----
  	  toc_initialized = 1;
  	}
        fprintf (asm_out_file, "\t.csect toc_table[RW]%s\n",
! 	       (TARGET_32BIT 
! 		? "" : ","XCOFF_CSECT_DEFAULT_ALIGNMENT_STR));
      }
    else
      fputs ("\t.toc\n", asm_out_file);
Index: testsuite/gnat.dg/oalign1.ads
===================================================================
*** testsuite/gnat.dg/oalign1.ads	(revision 0)
--- testsuite/gnat.dg/oalign1.ads	(revision 0)
***************
*** 0 ****
--- 1,5 ----
+ 
+ package Oalign1 is
+    Klunk1 : Integer := 12;
+    for Klunk1'Alignment use Standard'Maximum_Alignment;
+ end;
Index: testsuite/gnat.dg/oalign2.ads
===================================================================
*** testsuite/gnat.dg/oalign2.ads	(revision 0)
--- testsuite/gnat.dg/oalign2.ads	(revision 0)
***************
*** 0 ****
--- 1,5 ----
+ 
+ package Oalign2 is
+    Klunk2 : Integer := 12;
+    for Klunk2'Alignment use Standard'Maximum_Alignment;
+ end;
Index: test_oalign.adb
===================================================================
*** test_oalign.adb	(revision 0)
--- test_oalign.adb	(revision 0)
***************
*** 0 ****
--- 1,14 ----
+ -- { dg-do run }
+ 
+ with System.Storage_Elements; use System.Storage_Elements;
+ with Oalign1, Oalign2; use Oalign1, Oalign2;
+ 
+ procedure Test_Oalign is
+ begin
+    if Klunk1'Address mod Klunk1'Alignment /= 0 then
+       raise Program_Error;
+    end if;
+    if Klunk2'Address mod Klunk2'Alignment /= 0 then
+       raise Program_Error;
+    end if;
+ end;

[PING] fix hardcoded rs6000-xcoff csects alignment

[Olivier Hainque]

Hello,

   http://gcc.gnu.org/ml/gcc-patches/2007-06/msg01227.html

   > BIGGEST_ALIGNMENT was bumped to 16bytes some time ago and the csect
   > alignments remained untouched

Thanks in advance,

Olivier

Re: [PATCH] fix hardcoded rs6000-xcoff csects alignment

[David Edelsohn]

	* config/rs6000/xcoff.h (XCOFF_CSECT_DEFAULT_ALIGNMENT_STR): New
	internal macro. Default alignment factor for csect directives, chosen
	to match what BIGGEST_ALIGNMENT allows.
	(DATA_SECTION_ASM_OP): Use it.
	* config/rs6000/rs6000.c
	(rs6000_xcoff_output_readonly_section_asm_op):
	Use XCOFF_CSECT_DEFAULT_ALIGNMENT_STR.
	(rs6000_xcoff_output_readwrite_section_asm_op): Likewise.
	(rs6000_xcoff_output_toc_section_asm_op): Likewise.

This is okay, except for the last change.  The secondary TOC section
contains pointers and currently aligns to the pointer size based on
TARGET_32BIT.  No need to align that csect more strictly than 64 bits, and
especially not varying between 32 bits and 128 bits.

Thanks, David

Re: [PATCH] fix hardcoded rs6000-xcoff csects alignment

[Olivier Hainque]

David Edelsohn wrote:
> 	* config/rs6000/xcoff.h (XCOFF_CSECT_DEFAULT_ALIGNMENT_STR): New
> 	internal macro. Default alignment factor for csect directives, chosen
> 	to match what BIGGEST_ALIGNMENT allows.
> 	(DATA_SECTION_ASM_OP): Use it.
> 	* config/rs6000/rs6000.c
> 	(rs6000_xcoff_output_readonly_section_asm_op):
> 	Use XCOFF_CSECT_DEFAULT_ALIGNMENT_STR.
> 	(rs6000_xcoff_output_readwrite_section_asm_op): Likewise.
> 	(rs6000_xcoff_output_toc_section_asm_op): Likewise.
> 
> This is okay, except for the last change.  The secondary TOC section
> contains pointers and currently aligns to the pointer size based on
> TARGET_32BIT.  No need to align that csect more strictly than 64 bits, and
> especially not varying between 32 bits and 128 bits.

 OK, revs 127956 for the source changes & 127957 for the testcase.

 Thanks.