diff mbox

[rs6000] PR 66337: Improve Compliance with Power ABI

Message ID 56C5DDB9.1000607@linux.vnet.ibm.com
State New
Headers show

Commit Message

Kelvin Nilsen Feb. 18, 2016, 3:05 p.m. UTC
This patch has bootstrapped and tested on powerpc64le-unknown-linux-gnu and powerpc64be-unknown-linux-gnu (both 32-bit and 64-bit) and 
powerpc64-unknown-freebsd11.0 (big endian) with no regressions.  Is it ok to fix this on the trunk?

The problem described in PR66337 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D66337) is that compiling for PowerPC targets with the -malign-power command-line option results in invalid field offsets for certain structure members.   As identified in the problem report, this results from a macro definition present in both config/rs6000/{freebsd64,linux64}.h, which in both cases is introduced by the comment:

/* PowerPC64 Linux word-aligns FP doubles when -malign-power is given. */

I have consulted various ABI documents, including "64-bit PowerPC ELF Application Binary Interface Supplement 1.9" (http://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html), "Power Architecture 64-Bit ELF V2 ABI Specification" (https://members.openpowerfoundation.org/document/dl/576), and "Power
Architecture(R) 32-bit Application Binary Interface Supplement 1.0 - Linux(R) & Embedded" (https://www.power.org/documentation/power-architecture-32-bit-abi-supplement-1-0-embeddedlinuxunified/).  I have not been able to find any basis for this comment and thus am concluding that the comment and existing implementation are incorrect.

The implemented patch removes the comment and changes the macro definition so that field alignment calculations on 64-bit architectures ignore the -malign-power command-line option.  With this fix, the test case identified in the PR behaves as was expected by the submitter.
gcc/ChangeLog:

2016-02-17  Kelvin Nilsen  <kelvin@gcc.gnu.org>

	PR target/66337
	* config/rs6000/freebsd64.h: Remove erroneous comment and correct
        definition of ADJUST_FIELD_ALIGN macro.
	* config/rs6000/linux64.h: Remove erroneous comment and correct
        definition of ADJUST_FIELD_ALIGN macro.

gcc/testsuite/ChangeLog:

2016-02-17  Kelvin Nilsen  <kelvin@gcc.gnu.org>

	PR target/66337
	* g++.dg/pr66337-1.C: New test.
diff mbox

Patch

Index: gcc/testsuite/g++.dg/pr66337-1.C
===================================================================
--- gcc/testsuite/g++.dg/pr66337-1.C	(revision 0)
+++ gcc/testsuite/g++.dg/pr66337-1.C	(revision 233507)
@@ -0,0 +1,77 @@ 
+/* { dg-do run { target { powerpc*-*-* } } } */
+/* { dg-options "-std=c++11 -malign-power -O2" } */
+
+/* Power ABI for 32-bit and 64-bit compilers place the same alignment
+   restrictions on long longs and doubles. */
+
+typedef double _Tp;
+
+struct _Tp2 { 
+  char b;
+  int i;
+  char c;
+  long long l;
+  _Tp _M_t; 
+};
+
+extern _Tp2 tp2e;
+
+struct _ST1 {
+  char a;
+};
+
+struct _ST2 {
+  int b;
+};
+
+struct _ST3 {
+  float f;
+};
+
+struct _ST4 {
+  double d;
+};
+
+struct _ST5 {
+  char a;
+  double d;
+};
+
+struct _ST6 {
+  double d;
+  char a;
+};
+
+int main ()
+{
+  int a = alignof (_Tp2);
+  int b = __alignof__(_Tp2::_M_t);
+  int c = alignof(_Tp);
+  int d = __alignof__(tp2e._M_t);
+  int e = alignof(_Tp2::_M_t);
+
+  int f = __alignof__(_Tp2::l);
+  int g = alignof (long long);
+  int h = __alignof__(tp2e.l);
+  int i = alignof(_Tp2::l);
+
+  if ((a != 8) || (b != 8) || (c != 8) || (d != 8) || (e != 8) 
+      || (f != 8) || (g != 8) || (h != 8) || (i != 8))
+    return -1;
+
+  a = sizeof (_ST1);
+  b = sizeof (_ST2);
+  c = sizeof (_ST3);
+  d = sizeof (_ST4);
+  e = sizeof (_ST5);
+  f = sizeof (_ST6);
+  g = sizeof (_Tp2);
+
+  if ((a != 1) || (b != 4) || (c != 4) || (d != 8) 
+      || (e != 16) || (f != 16) || (g != 32))
+    return -2;
+
+  /* success */
+  return 0;
+}
+  
Index: gcc/config/rs6000/freebsd64.h
===================================================================
--- gcc/config/rs6000/freebsd64.h	(revision 233308)
+++ gcc/config/rs6000/freebsd64.h	(working copy)
@@ -363,16 +363,10 @@  extern int dot_symbols;
 /* Use standard DWARF numbering for DWARF debugging information.  */
 #define RS6000_USE_DWARF_NUMBERING
 
-/* PowerPC64 Linux word-aligns FP doubles when -malign-power is given.  */
 #undef  ADJUST_FIELD_ALIGN
 #define ADJUST_FIELD_ALIGN(FIELD, COMPUTED) \
   (rs6000_special_adjust_field_align_p ((FIELD), (COMPUTED))		\
-   ? 128                                                                \
-   : (TARGET_64BIT                                                      \
-      && TARGET_ALIGN_NATURAL == 0                                      \
-      && TYPE_MODE (strip_array_types (TREE_TYPE (FIELD))) == DFmode)   \
-   ? MIN ((COMPUTED), 32)                                               \
-   : (COMPUTED))
+   ? 128: (COMPUTED))
 
 #undef  TOC_SECTION_ASM_OP
 #define TOC_SECTION_ASM_OP \
Index: gcc/config/rs6000/linux64.h
===================================================================
--- gcc/config/rs6000/linux64.h	(revision 233308)
+++ gcc/config/rs6000/linux64.h	(working copy)
@@ -290,16 +290,10 @@  extern int dot_symbols;
 #define PROFILE_HOOK(LABEL) \
   do { if (TARGET_64BIT) output_profile_hook (LABEL); } while (0)
 
-/* PowerPC64 Linux word-aligns FP doubles when -malign-power is given.  */
 #undef  ADJUST_FIELD_ALIGN
 #define ADJUST_FIELD_ALIGN(FIELD, COMPUTED) \
   (rs6000_special_adjust_field_align_p ((FIELD), (COMPUTED))		\
-   ? 128								\
-   : (TARGET_64BIT							\
-      && TARGET_ALIGN_NATURAL == 0					\
-      && TYPE_MODE (strip_array_types (TREE_TYPE (FIELD))) == DFmode)	\
-   ? MIN ((COMPUTED), 32)						\
-   : (COMPUTED))
+   ? 128: (COMPUTED))
 
 /* PowerPC64 Linux increases natural record alignment to doubleword if
    the first field is an FP double, only if in power alignment mode.  */