This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Patch, MIPS] Patch for PR 68273 (user aligned variable arguments)


Here is a new patch for PR 68273.  The original problem with gsoap has
been fixed by changing GCC to not create overly-aligned variables in
the SRA pass but the MIPS specific problem of how user-aligned variables
are passed to functions remains.

Because MIPS GCC is internally inconsistent, it seems like changing
the passing convention for user aligned variables on MIPS is the best
option, even though this is an ABI change for MIPS.

For example:

	typedef int alignedint __attribute__((aligned(8)));
	int foo1 (int x, alignedint y) { return x+y; }
	int foo2 (int x, int y) { return x+y; }
	int foo3 (int x, alignedint y) { return x+y; }
	int foo4 (int x, int y) { return x+y; }
	int splat1(int a, alignedint b) { foo1(a,b); }
	int splat2(int a, alignedint b) { foo2(a,b); }
	int splat3(int a, int b) { foo3(a,b); }
	int splat4(int a, int b) { foo4(a,b); }

In this case foo1 and foo3 would expect the second argument to be in
register $6, but foo2 and foo4 would exect it in register $5.

Likewise splat1 and splat2 would pass the second argument in $6, but
splat3 and splat4 would pass it in $5.

This means that the call from splat2 to foo2 and the call from splat3
to foo3 would be wrong in that the caller is putting the argument in
one register but the callee is getting out of a different register.

In none of these cases would GCC give a warning about the inconsistent
parameter passing 

Since this patch could cause a change in the users program, I have
added a warning that will be emitted whenever a user passes an
aligned type or when a parameter is declared as an aligned type
and that alignment could cause a change in how the value is passed.

I also added a way to turn that warning off in case the user doesn't
want to see it (-mno-warn-aligned-args).  I did not add an option
to make GCC pass arguments in the old manner as I consider that method
of passing arguments to be a bug and I don't think we want to have an
option to propogate that incorrect behavior.

Steve Ellcey
sellcey@imgtec.com


2016-02-24  Steve Ellcey  <sellcey@imgtec.com>

	PR target/68273
	* config/mips/mips.opt (mwarn-aligned-args): New flag.
	* config/mips/mips.h (mips_args): Add new field.
	* config/mips/mips.c (mips_internal_function_arg): New function.
	(mips_function_incoming_arg): New function.
	(mips_old_function_arg_boundary): New function.
	(mips_function_arg): Rewrite to use mips_internal_function_arg.
	(mips_function_arg_boundary): Fix argument alignment.
	(TARGET_FUNCTION_INCOMING_ARG): New define.


diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 697abc2..05465c1 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -1124,6 +1124,7 @@ static const struct mips_rtx_cost_data
 static rtx mips_find_pic_call_symbol (rtx_insn *, rtx, bool);
 static int mips_register_move_cost (machine_mode, reg_class_t,
 				    reg_class_t);
+static unsigned int mips_old_function_arg_boundary (machine_mode, const_tree);
 static unsigned int mips_function_arg_boundary (machine_mode, const_tree);
 static machine_mode mips_get_reg_raw_mode (int regno);
 
@@ -5459,11 +5460,11 @@ mips_strict_argument_naming (cumulative_args_t ca ATTRIBUTE_UNUSED)
   return !TARGET_OLDABI;
 }
 
-/* Implement TARGET_FUNCTION_ARG.  */
+/* Used to implement TARGET_FUNCTION_ARG and TARGET_FUNCTION_INCOMING_ARG.  */
 
 static rtx
-mips_function_arg (cumulative_args_t cum_v, machine_mode mode,
-		   const_tree type, bool named)
+mips_internal_function_arg (cumulative_args_t cum_v, machine_mode mode,
+			    const_tree type, bool named)
 {
   CUMULATIVE_ARGS *cum = get_cumulative_args (cum_v);
   struct mips_arg_info info;
@@ -5586,6 +5587,50 @@ mips_function_arg (cumulative_args_t cum_v, machine_mode mode,
   return gen_rtx_REG (mode, mips_arg_regno (&info, TARGET_HARD_FLOAT));
 }
 
+/* Implement TARGET_FUNCTION_ARG.  */
+
+static rtx
+mips_function_arg (cumulative_args_t cum_v, machine_mode mode,
+		   const_tree type, bool named)
+{
+  bool doubleword_aligned_p = (mips_function_arg_boundary (mode, type)
+ 			  > BITS_PER_WORD);
+  bool old_doubleword_aligned_p = (mips_old_function_arg_boundary (mode, type)
+			      > BITS_PER_WORD);
+  CUMULATIVE_ARGS *cum = get_cumulative_args (cum_v);
+
+  if (doubleword_aligned_p != old_doubleword_aligned_p
+      && mips_warn_aligned_args && !cum->aligned_arg_warning_given)
+    {
+      warning (0, "argument %d in the call may be passed in a manner incompatible with previous GCC versions", cum->arg_number+1);
+      cum->aligned_arg_warning_given = 1;
+    }
+
+  return mips_internal_function_arg (cum_v, mode, type, named);
+}
+
+/* Implement TARGET_FUNCTION_INCOMING_ARG.  */
+
+static rtx
+mips_function_incoming_arg (cumulative_args_t cum_v, machine_mode mode,
+			    const_tree type, bool named)
+{
+  bool doubleword_aligned_p = (mips_function_arg_boundary (mode, type)
+ 			  > BITS_PER_WORD);
+  bool old_doubleword_aligned_p = (mips_old_function_arg_boundary (mode, type)
+			      > BITS_PER_WORD);
+  CUMULATIVE_ARGS *cum = get_cumulative_args (cum_v);
+
+  if (doubleword_aligned_p != old_doubleword_aligned_p
+      && mips_warn_aligned_args && !cum->aligned_arg_warning_given)
+    {
+      warning (0, "parameter %d of function %q+F may be passed in a manner incompatible with previous GCC versions", cum->arg_number+1, current_function_decl);
+      cum->aligned_arg_warning_given = 1;
+    }
+
+  return mips_internal_function_arg (cum_v, mode, type, named);
+}
+
 /* Implement TARGET_FUNCTION_ARG_ADVANCE.  */
 
 static void
@@ -5635,6 +5680,23 @@ mips_arg_partial_bytes (cumulative_args_t cum,
   return info.stack_words > 0 ? info.reg_words * UNITS_PER_WORD : 0;
 }
 
+/* This is the old TARGET_FUNCTION_ARG_BOUNDARY, we use it to see if there
+   is a change from the new one so we can warn users.  */
+
+static unsigned int
+mips_old_function_arg_boundary (machine_mode mode, const_tree type)
+{
+  unsigned int alignment;
+
+  alignment = type ? TYPE_ALIGN (type) : GET_MODE_ALIGNMENT (mode);
+
+  if (alignment < PARM_BOUNDARY)
+    alignment = PARM_BOUNDARY;
+  if (alignment > STACK_BOUNDARY)
+    alignment = STACK_BOUNDARY;
+  return alignment;
+}
+
 /* Implement TARGET_FUNCTION_ARG_BOUNDARY.  Every parameter gets at
    least PARM_BOUNDARY bits of alignment, but will be given anything up
    to STACK_BOUNDARY bits if the type requires it.  */
@@ -5644,7 +5706,10 @@ mips_function_arg_boundary (machine_mode mode, const_tree type)
 {
   unsigned int alignment;
 
-  alignment = type ? TYPE_ALIGN (type) : GET_MODE_ALIGNMENT (mode);
+  alignment = type
+		? TYPE_ALIGN (TYPE_MAIN_VARIANT (type))
+		: GET_MODE_ALIGNMENT (mode);
+
   if (alignment < PARM_BOUNDARY)
     alignment = PARM_BOUNDARY;
   if (alignment > STACK_BOUNDARY)
@@ -5652,6 +5717,7 @@ mips_function_arg_boundary (machine_mode mode, const_tree type)
   return alignment;
 }
 
+
 /* Implement TARGET_GET_RAW_RESULT_MODE and TARGET_GET_RAW_ARG_MODE.  */
 
 static machine_mode
@@ -20108,6 +20174,8 @@ mips_promote_function_mode (const_tree type ATTRIBUTE_UNUSED,
 #define TARGET_ARG_PARTIAL_BYTES mips_arg_partial_bytes
 #undef TARGET_FUNCTION_ARG
 #define TARGET_FUNCTION_ARG mips_function_arg
+#undef TARGET_FUNCTION_INCOMING_ARG
+#define TARGET_FUNCTION_INCOMING_ARG mips_function_incoming_arg
 #undef TARGET_FUNCTION_ARG_ADVANCE
 #define TARGET_FUNCTION_ARG_ADVANCE mips_function_arg_advance
 #undef TARGET_FUNCTION_ARG_BOUNDARY
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 803ab98..c836a23 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -2460,6 +2460,10 @@ typedef struct mips_args {
 
   /* True if the function has a prototype.  */
   int prototype;
+
+  /* True if we have warned about possible change in how aligned arguments
+     are passed to functions. */
+  int aligned_arg_warning_given;
 } CUMULATIVE_ARGS;
 
 /* Initialize a variable CUM of type CUMULATIVE_ARGS
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index ebd67e4..d6f486d 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -396,6 +396,10 @@ mvirt
 Target Report Var(TARGET_VIRT)
 Use Virtualization Application Specific instructions.
 
+mwarn-aligned-args
+Target Report Var(mips_warn_aligned_args) Init(1)
+Warn if an aligned argument is being passed in a manner incomatible with older GCC versions.
+
 mxpa
 Target Report Var(TARGET_XPA)
 Use eXtended Physical Address (XPA) instructions.



2016-02-24  Steve Ellcey  <sellcey@imgtec.com>

	PR target/68273
	* gcc.c-torture/execute/pr68273-1.c: New test.
	* gcc.c-torture/execute/pr68273-2.c: New test.
	* gcc.target/mips/pr68273-warn.c: New test. 
	* gcc.dg/pr48335-2.c: Turn off warning on MIPS.
	* gcc.dg/torture/pr48493.c: Ditto.


diff --git a/gcc/testsuite/gcc.c-torture/execute/pr68273-1.c b/gcc/testsuite/gcc.c-torture/execute/pr68273-1.c
index e69de29..7cfd654 100644
--- a/gcc/testsuite/gcc.c-torture/execute/pr68273-1.c
+++ b/gcc/testsuite/gcc.c-torture/execute/pr68273-1.c
@@ -0,0 +1,76 @@
+/* { dg-options "-mno-warn-aligned-args" { target mips*-*-* } } */
+
+/* Make sure that the alignment attribute on an argument passed by
+   value does not affect the calling convention and what registers
+   arguments are passed in.  */
+
+extern void exit (int);
+extern void abort (void);
+
+typedef int alignedint __attribute__((aligned(8)));
+
+int  __attribute__((noinline))
+foo1 (int a, alignedint b)
+{ return a + b; }
+
+int __attribute__((noinline))
+foo2 (int a, int b)
+{
+  return a + b;
+}
+
+int __attribute__((noinline))
+bar1 (alignedint x)
+{
+  return foo1 (1, x);
+}
+
+int __attribute__((noinline))
+bar2 (alignedint x)
+{
+  return foo1 (1, (alignedint) 99);
+}
+
+int __attribute__((noinline))
+bar3 (alignedint x)
+{
+  return foo1 (1, x + (alignedint) 1);
+}
+
+alignedint q = 77;
+
+int __attribute__((noinline))
+bar4 (alignedint x)
+{
+  return foo1 (1, q);
+}
+
+
+int __attribute__((noinline))
+bar5 (alignedint x)
+{
+  return foo2 (1, x);
+}
+
+int __attribute__((noinline))
+use_arg_regs (int i, int j, int k)
+{
+  return i+j-k;
+}
+
+int main()
+{
+   if (use_arg_regs (999, 999, 999) != 999) abort ();
+   if (foo1 (19,13) != 32) abort ();
+   if (use_arg_regs (999, 999, 999) != 999) abort ();
+   if (bar1 (-33) != -32) abort ();
+   if (use_arg_regs (999, 999, 999) != 999) abort ();
+   if (bar2 (1) != 100) abort ();
+   if (use_arg_regs (999, 999, 999) != 999) abort ();
+   if (bar3 (17) != 19) abort ();
+   if (use_arg_regs (999, 999, 999) != 999) abort ();
+   if (bar4 (-33) != 78) abort ();
+   if (use_arg_regs (999, 999, 999) != 999) abort ();
+   if (bar5 (-84) != -83) abort ();
+   exit (0);
+}
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr68273-2.c b/gcc/testsuite/gcc.c-torture/execute/pr68273-2.c
index e69de29..157dbfe 100644
--- a/gcc/testsuite/gcc.c-torture/execute/pr68273-2.c
+++ b/gcc/testsuite/gcc.c-torture/execute/pr68273-2.c
@@ -0,0 +1,111 @@
+/* { dg-options "-mno-warn-aligned-args" { target mips*-*-* } } */
+
+/* Make sure that the alignment attribute on an argument passed by
+   value does not affect the calling convention and what registers
+   arguments are passed in.  */
+
+extern void exit (int);
+extern void abort (void);
+
+typedef struct s {
+	char c;
+	char d;
+} t1;
+typedef t1 t1_aligned8  __attribute__((aligned(8)));
+typedef t1 t1_aligned16 __attribute__((aligned(16)));
+typedef t1 t1_aligned32 __attribute__((aligned(32)));
+
+int bar1(int a, t1 b)
+{
+	return a + b.c + b.d;
+}
+
+int bar2(int a, t1_aligned8 b)
+{
+	return a + b.c + b.d;
+}
+
+int bar3(int a, t1_aligned16 b)
+{
+	return a + b.c + b.d;
+}
+
+int bar4(int a, t1_aligned32 b)
+{
+	return a + b.c + b.d;
+}
+
+#define FOODEF(n,m,type) \
+int __attribute__((noinline)) \
+foo##n (int i, type b) \
+  { \
+    return bar##m (i, b); \
+  }
+
+FOODEF(1,  1, t1)
+FOODEF(2,  1, t1_aligned8)
+FOODEF(3,  1, t1_aligned16)
+FOODEF(4,  1, t1_aligned32)
+FOODEF(5,  2, t1)
+FOODEF(6,  2, t1_aligned8)
+FOODEF(7,  2, t1_aligned16)
+FOODEF(8,  2, t1_aligned32)
+FOODEF(9,  3, t1)
+FOODEF(10, 3, t1_aligned8)
+FOODEF(11, 3, t1_aligned16)
+FOODEF(12, 3, t1_aligned32)
+FOODEF(13, 4, t1)
+FOODEF(14, 4, t1_aligned8)
+FOODEF(15, 4, t1_aligned16)
+FOODEF(16, 4, t1_aligned32)
+
+int __attribute__((noinline))
+use_arg_regs (int i, int j, int k)
+{
+  return i+j-k;
+}
+
+#define FOOCALL(i) \
+  c = i*11 + 39; \
+  x1.c = i + 5; \
+  x1.d = i*2 + 3; \
+  x2.c = x1.c + 1; \
+  x2.d = x1.d + 1; \
+  x3.c = x2.c + 1; \
+  x3.d = x2.d + 1; \
+  x4.c = x3.c + 1; \
+  x4.d = x3.d + 1; \
+  if (use_arg_regs (999,999,999) != 999) abort (); \
+  if (foo##i (c, x1) != c + x1.c + x1.d) abort (); \
+  if (use_arg_regs (999,999,999) != 999) abort (); \
+  if (foo##i (c, x2) != c + x2.c + x2.d) abort (); \
+  if (use_arg_regs (999,999,999) != 999) abort (); \
+  if (foo##i (c, x3) != c + x3.c + x3.d) abort (); \
+  if (use_arg_regs (999,999,999) != 999) abort (); \
+  if (foo##i (c, x4) != c + x4.c + x4.d) abort ();
+
+int main()
+{
+  int c;
+  t1 x1;
+  t1_aligned8 x2;
+  t1_aligned16 x3;
+  t1_aligned32 x4;
+  FOOCALL(1);
+  FOOCALL(2);
+  FOOCALL(3);
+  FOOCALL(4);
+  FOOCALL(5);
+  FOOCALL(6);
+  FOOCALL(7);
+  FOOCALL(8);
+  FOOCALL(9);
+  FOOCALL(10);
+  FOOCALL(11);
+  FOOCALL(12);
+  FOOCALL(13);
+  FOOCALL(14);
+  FOOCALL(15);
+  FOOCALL(16);
+  exit (0);
+}
diff --git a/gcc/testsuite/gcc.dg/pr48335-2.c b/gcc/testsuite/gcc.dg/pr48335-2.c
index a37c079..f56b6fd 100644
--- a/gcc/testsuite/gcc.dg/pr48335-2.c
+++ b/gcc/testsuite/gcc.dg/pr48335-2.c
@@ -1,6 +1,7 @@
 /* PR middle-end/48335 */
 /* { dg-do compile } */
 /* { dg-options "-O2 -fno-tree-sra" } */
+/* { dg-options "-O2 -fno-tree-sra -mno-warn-aligned-args" { target mips*-*-* } } */
 
 typedef long long T __attribute__((may_alias, aligned (1)));
 typedef short U __attribute__((may_alias, aligned (1)));
diff --git a/gcc/testsuite/gcc.dg/torture/pr48493.c b/gcc/testsuite/gcc.dg/torture/pr48493.c
index ddb61f2..77f30c7 100644
--- a/gcc/testsuite/gcc.dg/torture/pr48493.c
+++ b/gcc/testsuite/gcc.dg/torture/pr48493.c
@@ -1,4 +1,5 @@
 /* { dg-do compile } */
+/* { dg-options "-mno-warn-aligned-args" { target mips*-*-* } } */
 
 typedef long long T __attribute__((may_alias, aligned (1)));
 
diff --git a/gcc/testsuite/gcc.target/mips/pr68273-warn.c b/gcc/testsuite/gcc.target/mips/pr68273-warn.c
index e69de29..f1b0dc8 100644
--- a/gcc/testsuite/gcc.target/mips/pr68273-warn.c
+++ b/gcc/testsuite/gcc.target/mips/pr68273-warn.c
@@ -0,0 +1,32 @@
+/* { dg-do compile } */
+/* { dg-skip-if "needs asm output" { *-*-* } { "-fno-fat-lto-objects" } { "" } } */
+
+typedef int alignedint __attribute__((aligned(8)));
+extern void foo1 (alignedint y);
+extern void foo2 (int y);
+extern void foo3 (alignedint y);
+extern void foo4 (int y);
+
+int
+splat1(alignedint b) /* { dg-warning "incompatible with previous GCC versions" } */
+{
+  foo1(b); /* { dg-warning "incompatible with previous GCC versions" } */
+}
+
+int
+splat2(alignedint b) /* { dg-warning "incompatible with previous GCC versions" } */
+{
+  foo2(b); /* { dg-warning "incompatible with previous GCC versions" } */
+}
+
+int
+splat3(int b)
+{
+  foo3(b);
+}
+
+int
+splat4(int b)
+{
+  foo4(b);
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]