Fix passing of > 8 byte aligned TYPE_EMPTY_P args (PR middle-end/83487)

Message ID 20171221203825.GF2353@tucnak
State New
Headers show
Series
  • Fix passing of > 8 byte aligned TYPE_EMPTY_P args (PR middle-end/83487)
Related show

Commit Message

Jakub Jelinek Dec. 21, 2017, 8:38 p.m.
Hi!

When we don't pass an argument at all because it is TYPE_EMPTY_P, we
shouldn't tweak argument slot alignment based on the alignment of these
arguments either.

This patch fixes the ICE we issued on pr83487.{c,C}.
As the compat tests show, we are now ABI compatible with clang++ trunk
for TYPE_EMPTY_P arguments with size <= 16 bytes, the alignment is ignored
when passing those; the arguments are NO_CLASS, NO_CLASS and the psABI
then doesn't say anything on passing them anywhere.  In C we are compatible
on all the tests because all the structs have actually zero size (but the
alignment is still ignored).  The case where we are incompatible on is
> 16 bytes TYPE_EMPTY_P, we need to decide if want to do it anyway and

update the psABI for that, or if we follow strictly the current psABI.
But in that case the fix would still be to just not set TYPE_EMPTY_P on
the larger empty structures (what is in pr83487_2*.C), and the
ix86_function_arg_boundary change would still work.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-12-21  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/83487
	* config/i386/i386.c (ix86_function_arg_boundary): Return
	PARM_BOUNDARY for TYPE_EMPTY_P types.

	* gcc.c-torture/compile/pr83487.c: New test.
	* gcc.dg/compat/pr83487-1.h: New file.
	* gcc.dg/compat/pr83487-1_main.c: New test.
	* gcc.dg/compat/pr83487-1_x.c: New file.
	* gcc.dg/compat/pr83487-1_y.c: New file.
	* gcc.dg/compat/pr83487-2_main.c: New test.
	* gcc.dg/compat/pr83487-2_x.c: New file.
	* gcc.dg/compat/pr83487-2_y.c: New file.
	* g++.dg/abi/pr83487.C: New test.
	* g++.dg/compat/abi/pr83487-1_main.C: New test.
	* g++.dg/compat/abi/pr83487-1_x.C: New file.
	* g++.dg/compat/abi/pr83487-1_y.C: New file.
	* g++.dg/compat/abi/pr83487-2_main.C: New test.
	* g++.dg/compat/abi/pr83487-2_x.C: New file.
	* g++.dg/compat/abi/pr83487-2_y.C: New file.


	Jakub

Comments

Jeff Law Dec. 21, 2017, 11:07 p.m. | #1
On 12/21/2017 01:38 PM, Jakub Jelinek wrote:
> Hi!

> 

> When we don't pass an argument at all because it is TYPE_EMPTY_P, we

> shouldn't tweak argument slot alignment based on the alignment of these

> arguments either.

> 

> This patch fixes the ICE we issued on pr83487.{c,C}.

> As the compat tests show, we are now ABI compatible with clang++ trunk

> for TYPE_EMPTY_P arguments with size <= 16 bytes, the alignment is ignored

> when passing those; the arguments are NO_CLASS, NO_CLASS and the psABI

> then doesn't say anything on passing them anywhere.  In C we are compatible

> on all the tests because all the structs have actually zero size (but the

> alignment is still ignored).  The case where we are incompatible on is

>> 16 bytes TYPE_EMPTY_P, we need to decide if want to do it anyway and

> update the psABI for that, or if we follow strictly the current psABI.

> But in that case the fix would still be to just not set TYPE_EMPTY_P on

> the larger empty structures (what is in pr83487_2*.C), and the

> ix86_function_arg_boundary change would still work.

> 

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

> 

> 2017-12-21  Jakub Jelinek  <jakub@redhat.com>

> 

> 	PR middle-end/83487

> 	* config/i386/i386.c (ix86_function_arg_boundary): Return

> 	PARM_BOUNDARY for TYPE_EMPTY_P types.

> 

> 	* gcc.c-torture/compile/pr83487.c: New test.

> 	* gcc.dg/compat/pr83487-1.h: New file.

> 	* gcc.dg/compat/pr83487-1_main.c: New test.

> 	* gcc.dg/compat/pr83487-1_x.c: New file.

> 	* gcc.dg/compat/pr83487-1_y.c: New file.

> 	* gcc.dg/compat/pr83487-2_main.c: New test.

> 	* gcc.dg/compat/pr83487-2_x.c: New file.

> 	* gcc.dg/compat/pr83487-2_y.c: New file.

> 	* g++.dg/abi/pr83487.C: New test.

> 	* g++.dg/compat/abi/pr83487-1_main.C: New test.

> 	* g++.dg/compat/abi/pr83487-1_x.C: New file.

> 	* g++.dg/compat/abi/pr83487-1_y.C: New file.

> 	* g++.dg/compat/abi/pr83487-2_main.C: New test.

> 	* g++.dg/compat/abi/pr83487-2_x.C: New file.

> 	* g++.dg/compat/abi/pr83487-2_y.C: New file.

OK
jeff

Patch

--- gcc/config/i386/i386.c.jj	2017-12-21 09:44:34.000000000 +0100
+++ gcc/config/i386/i386.c	2017-12-21 13:04:03.172252517 +0100
@@ -8973,6 +8973,8 @@  ix86_function_arg_boundary (machine_mode
 	 the main variant type.  */
       type = TYPE_MAIN_VARIANT (type);
       align = TYPE_ALIGN (type);
+      if (TYPE_EMPTY_P (type))
+	return PARM_BOUNDARY;
     }
   else
     align = GET_MODE_ALIGNMENT (mode);
--- gcc/testsuite/gcc.c-torture/compile/pr83487.c.jj	2017-12-21 19:29:54.306582133 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr83487.c	2017-12-21 19:29:47.585666846 +0100
@@ -0,0 +1,11 @@ 
+/* PR middle-end/83487 */
+
+struct __attribute__ ((aligned)) A {};
+struct A a;
+void bar (int, int, int, int, int, int, int, struct A);
+
+void
+foo (void)
+{
+  bar (0, 1, 2, 3, 4, 5, 6, a);
+}
--- gcc/testsuite/gcc.dg/compat/pr83487-1.h.jj	2017-12-21 19:35:55.883024640 +0100
+++ gcc/testsuite/gcc.dg/compat/pr83487-1.h	2017-12-21 19:46:30.100022814 +0100
@@ -0,0 +1,9 @@ 
+#ifdef PR83487_LARGE
+struct __attribute__ ((aligned (128))) A {};
+struct B {};
+struct C { struct B c[128]; };
+#else
+struct __attribute__ ((aligned (16))) A {};
+struct B {};
+struct C { struct B c[16]; };
+#endif
--- gcc/testsuite/gcc.dg/compat/pr83487-1_main.c.jj	2017-12-21 19:35:44.245171329 +0100
+++ gcc/testsuite/gcc.dg/compat/pr83487-1_main.c	2017-12-21 19:35:39.941225579 +0100
@@ -0,0 +1,8 @@ 
+extern void do_test (void);
+
+int
+main ()
+{
+  do_test ();
+  return 0;
+}
--- gcc/testsuite/gcc.dg/compat/pr83487-1_x.c.jj	2017-12-21 19:35:47.544129748 +0100
+++ gcc/testsuite/gcc.dg/compat/pr83487-1_x.c	2017-12-21 19:46:39.601902712 +0100
@@ -0,0 +1,63 @@ 
+#include "pr83487-1.h"
+
+extern
+#ifdef __cplusplus
+"C"
+#endif
+void abort ();
+
+void
+f1 (int i, int j, int k, int l, int m, int n, int o, struct A x)
+{
+  if (i != 6 || j != 0 || k != 1 || l != 2 || m != 3 || n != 4 || o != 5)
+    abort ();
+}
+
+void
+f2 (int i, int j, int k, int l, int m, int n, int o, struct A x, int p, int q)
+{
+  if (i != 6 || j != 0 || k != 1 || l != 2 || m != 3 || n != 4 || o != 5 || p != 7 || q != 8)
+    abort ();
+}
+
+void
+f3 (int i, int j, int k, int l, int m, int n, int o, struct B x, int p, int q)
+{
+  if (i != 6 || j != 0 || k != 1 || l != 2 || m != 3 || n != 4 || o != 5 || p != 7 || q != 8)
+    abort ();
+}
+
+void
+f4 (int i, int j, int k, int l, int m, int n, int o, struct C x, int p, int q)
+{
+  if (i != 6 || j != 0 || k != 1 || l != 2 || m != 3 || n != 4 || o != 5 || p != 7 || q != 8)
+    abort ();
+}
+
+void
+f5 (int o, struct A x)
+{
+  if (o != 5)
+    abort ();
+}
+
+void
+f6 (int o, struct A x, int p, int q)
+{
+  if (o != 5 || p != 7 || q != 8)
+    abort ();
+}
+
+void
+f7 (int o, struct B x, int p, int q)
+{
+  if (o != 5 || p != 7 || q != 8)
+    abort ();
+}
+
+void
+f8 (int o, struct C x, int p, int q)
+{
+  if (o != 5 || p != 7 || q != 8)
+    abort ();
+}
--- gcc/testsuite/gcc.dg/compat/pr83487-1_y.c.jj	2017-12-21 19:35:51.103084889 +0100
+++ gcc/testsuite/gcc.dg/compat/pr83487-1_y.c	2017-12-21 19:37:05.604145840 +0100
@@ -0,0 +1,27 @@ 
+#include "pr83487-1.h"
+
+struct A a;
+struct B b;
+struct C c;
+
+extern void f1 (int i, int j, int k, int l, int m, int n, int o, struct A);
+extern void f2 (int i, int j, int k, int l, int m, int n, int o, struct A, int p, int q);
+extern void f3 (int i, int j, int k, int l, int m, int n, int o, struct B, int p, int q);
+extern void f4 (int i, int j, int k, int l, int m, int n, int o, struct C, int p, int q);
+extern void f5 (int o, struct A);
+extern void f6 (int o, struct A, int p, int q);
+extern void f7 (int o, struct B, int p, int q);
+extern void f8 (int o, struct C, int p, int q);
+
+void
+do_test ()
+{
+  f1 (6, 0, 1, 2, 3, 4, 5, a);
+  f2 (6, 0, 1, 2, 3, 4, 5, a, 7, 8);
+  f3 (6, 0, 1, 2, 3, 4, 5, b, 7, 8);
+  f4 (6, 0, 1, 2, 3, 4, 5, c, 7, 8);
+  f5 (5, a);
+  f6 (5, a, 7, 8);
+  f7 (5, b, 7, 8);
+  f8 (5, c, 7, 8);
+}
--- gcc/testsuite/gcc.dg/compat/pr83487-2_main.c.jj	2017-12-21 19:45:03.600116158 +0100
+++ gcc/testsuite/gcc.dg/compat/pr83487-2_main.c	2017-12-21 19:47:09.175528906 +0100
@@ -0,0 +1 @@ 
+#include "pr83487-1_main.c"
--- gcc/testsuite/gcc.dg/compat/pr83487-2_x.c.jj	2017-12-21 19:45:07.284069594 +0100
+++ gcc/testsuite/gcc.dg/compat/pr83487-2_x.c	2017-12-21 19:47:30.545258796 +0100
@@ -0,0 +1,2 @@ 
+#define PR83487_LARGE
+#include "pr83487-1_x.c"
--- gcc/testsuite/gcc.dg/compat/pr83487-2_y.c.jj	2017-12-21 19:45:10.446029627 +0100
+++ gcc/testsuite/gcc.dg/compat/pr83487-2_y.c	2017-12-21 19:47:42.538107209 +0100
@@ -0,0 +1,2 @@ 
+#define PR83487_LARGE
+#include "pr83487-1_y.c"
--- gcc/testsuite/g++.dg/abi/pr83487.C.jj	2017-12-21 19:30:46.921918942 +0100
+++ gcc/testsuite/g++.dg/abi/pr83487.C	2017-12-21 19:31:17.385534963 +0100
@@ -0,0 +1,13 @@ 
+/* PR middle-end/83487 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+struct __attribute__ ((__aligned__)) A {};
+struct A a;
+void bar (int, int, int, int, int, int, int, struct A);
+
+void
+foo ()
+{
+  bar (0, 1, 2, 3, 4, 5, 6, a);
+}
--- gcc/testsuite/g++.dg/compat/abi/pr83487-1_main.C.jj	2017-12-21 19:50:29.042002628 +0100
+++ gcc/testsuite/g++.dg/compat/abi/pr83487-1_main.C	2017-12-21 19:35:39.000000000 +0100
@@ -0,0 +1,8 @@ 
+extern void do_test (void);
+
+int
+main ()
+{
+  do_test ();
+  return 0;
+}
--- gcc/testsuite/g++.dg/compat/abi/pr83487-1_x.C.jj	2017-12-21 19:51:49.405986840 +0100
+++ gcc/testsuite/g++.dg/compat/abi/pr83487-1_x.C	2017-12-21 19:51:01.592591194 +0100
@@ -0,0 +1 @@ 
+#include "../../../gcc.dg/compat/pr83487-1_x.c"
--- gcc/testsuite/g++.dg/compat/abi/pr83487-1_y.C.jj	2017-12-21 19:51:49.405986840 +0100
+++ gcc/testsuite/g++.dg/compat/abi/pr83487-1_y.C	2017-12-21 19:51:56.239900461 +0100
@@ -0,0 +1 @@ 
+#include "../../../gcc.dg/compat/pr83487-1_y.c"
--- gcc/testsuite/g++.dg/compat/abi/pr83487-2_main.C.jj	2017-12-21 19:50:32.151963318 +0100
+++ gcc/testsuite/g++.dg/compat/abi/pr83487-2_main.C	2017-12-21 19:53:07.126004473 +0100
@@ -0,0 +1,8 @@ 
+extern void do_test (void);
+
+int
+main ()
+{
+  do_test ();
+  return 0;
+}
--- gcc/testsuite/g++.dg/compat/abi/pr83487-2_x.C.jj	2017-12-21 19:51:49.405986840 +0100
+++ gcc/testsuite/g++.dg/compat/abi/pr83487-2_x.C	2017-12-21 19:52:03.695806220 +0100
@@ -0,0 +1 @@ 
+#include "../../../gcc.dg/compat/pr83487-2_x.c"
--- gcc/testsuite/g++.dg/compat/abi/pr83487-2_y.C.jj	2017-12-21 19:51:49.405986840 +0100
+++ gcc/testsuite/g++.dg/compat/abi/pr83487-2_y.C	2017-12-21 19:52:09.639731089 +0100
@@ -0,0 +1 @@ 
+#include "../../../gcc.dg/compat/pr83487-2_y.c"