public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Steve Ellcey " <sellcey@imgtec.com>
To: <gcc-patches@gcc.gnu.org>
Subject: [Patch, MIPS] Fix PR target/68273, passing args in wrong regs
Date: Fri, 29 Jan 2016 20:37:00 -0000	[thread overview]
Message-ID: <a80cfc11-c2a6-4142-b66f-a522f227d6ba@BAMAIL02.ba.imgtec.org> (raw)


This is a patch for PR 68273, where MIPS is passing arguments in the wrong
registers.  The problem is that when passing an argument by value,
mips_function_arg_boundary was looking at the type of the argument (and
not just the mode), and if that argument was a variable with extra alignment
info (say an integer variable with 8 byte alignment), MIPS was aligning the
argument on an 8 byte boundary instead of a 4 byte boundary the way it should.

Since we are passing values (not variables), the alignment of the variable
that the value is copied from should not affect the alignment of the value
being passed.

This patch fixes the problem and it could change what registers arguments
are passed in, which means it could affect backwards compatibility with
older programs.  But since the current behaviour is not compliant with the
MIPS ABI and does not match what LLVM does, I think we have to make this
change.  For the most part this will only affect arguments which are copied
from variables that have non-standard alignments set by the aligned attribute,
however the SRA optimization pass can create aligned variables as it splits
aggregates apart and that was what triggered this bug report.

This is basically the same bug as the ARM bug PR 65956 and the fix is
pretty much the same too.

Rather than create MIPS specific tests that check the use of specific
registers I created two tests to put in gcc.c-torture/execute that
were failing before because GCC on MIPS was not consistent in where
arguments were passed and which now work with this patch.

Tested with mips-mti-linux-gnu and no regressions.  OK to checkin?

Steve Ellcey
sellcey@imgtec.com


2016-01-29  Steve Ellcey  <sellcey@imgtec.com>

	PR target/68273
	* config/mips/mips.c (mips_function_arg_boundary): Fix argument
	alignment.



diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index dd54d6a..ecce3cd 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -5643,8 +5643,9 @@ static unsigned int
 mips_function_arg_boundary (machine_mode mode, const_tree type)
 {
   unsigned int alignment;
-
-  alignment = type ? TYPE_ALIGN (type) : GET_MODE_ALIGNMENT (mode);
+  alignment = type && mode == BLKmode
+	      ? TYPE_ALIGN (TYPE_MAIN_VARIANT (type))
+	      : GET_MODE_ALIGNMENT (mode);
   if (alignment < PARM_BOUNDARY)
     alignment = PARM_BOUNDARY;
   if (alignment > STACK_BOUNDARY)


2016-01-29  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.


diff --git a/gcc/testsuite/gcc.c-torture/execute/pr68273-1.c b/gcc/testsuite/gcc.c-torture/execute/pr68273-1.c
index e69de29..3ce07c6 100644
--- a/gcc/testsuite/gcc.c-torture/execute/pr68273-1.c
+++ b/gcc/testsuite/gcc.c-torture/execute/pr68273-1.c
@@ -0,0 +1,74 @@
+/* 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..1661be9 100644
--- a/gcc/testsuite/gcc.c-torture/execute/pr68273-2.c
+++ b/gcc/testsuite/gcc.c-torture/execute/pr68273-2.c
@@ -0,0 +1,109 @@
+/* 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);
+}

             reply	other threads:[~2016-01-29 20:37 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-29 20:37 Steve Ellcey  [this message]
2016-01-30 11:06 ` Richard Sandiford
2016-02-01 12:00   ` Richard Biener
2016-02-01 12:31     ` Eric Botcazou
2016-02-03 22:30     ` Steve Ellcey
2016-02-04  0:29       ` Mike Stump
2016-02-04 11:09         ` Eric Botcazou
2016-02-04 12:49           ` Richard Biener
2016-02-04 12:59             ` Richard Biener
2016-02-04 20:28             ` Richard Sandiford
2016-02-04 22:04               ` Richard Biener
2016-02-05 11:43             ` Eric Botcazou
2016-02-04 18:26           ` Steve Ellcey
2016-02-01 21:50   ` Steve Ellcey
2016-02-02 23:10   ` Steve Ellcey
2016-02-03  9:07     ` Eric Botcazou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a80cfc11-c2a6-4142-b66f-a522f227d6ba@BAMAIL02.ba.imgtec.org \
    --to=sellcey@imgtec.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).