diff mbox

Fix ICE with x87 asm operands (PR inline-asm/68843)

Message ID HE1PR07MB1580427A1FF65815CB85A023E44D0@HE1PR07MB1580.eurprd07.prod.outlook.com
State New
Headers show

Commit Message

Bernd Edlinger May 22, 2016, 7 a.m. UTC 
Hi!

as described in the PR there are several non-intuitive rules that
one has to follow to avoid ICEs with x87 asm operands.

This patch adds an explicit rule, that avoids ICE in the first test case and
removes an unnecessary error message in the second test case.


Boot-strapped and regression-tested on x86_64-pc-linux-gnu.
OK for trunk?


Thanks
Bernd.
gcc:
2016-05-22  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR inline-asm/68843
	* reg-stack.c (check_asm_stack_operands): Explicit input arguments
	must be grouped on top of stack.  Don't force early clobber
	on ordinary reg outputs.

testsuite:
2016-05-22  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR inline-asm/68843
	* gcc.target/i386/pr68843-1.c: New test.
	* gcc.target/i386/pr68843-2.c: New test.

Comments

Uros Bizjak May 22, 2016, 8:02 p.m. UTC | #1 
On Sun, May 22, 2016 at 9:00 AM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> Hi!
>
> as described in the PR there are several non-intuitive rules that
> one has to follow to avoid ICEs with x87 asm operands.
>
> This patch adds an explicit rule, that avoids ICE in the first test case and
> removes an unnecessary error message in the second test case.
>
>
> Boot-strapped and regression-tested on x86_64-pc-linux-gnu.
> OK for trunk?

This patch is actually dealing with two separate problems

This part:

@@ -607,7 +631,7 @@ check_asm_stack_operands (rtx_insn *insn)
      record any earlyclobber.  */

   for (i = n_outputs; i < n_outputs + n_inputs; i++)
-    if (op_alt[i].matches == -1)
+    if (op_alt[i].matches == -1 && STACK_REG_P (recog_data.operand[i]))
       {
  int j;

is OK, although, I'd written it as:


+    if (STACK_REG_P (recog_data.operand[i]) && op_alt[i].matches == -1)

with slightly simplified testcase:

--cut here--
int
__attribute__((noinline, noclone))
test (double y)
{
  int a, b;
  asm ("fistpl (%1)\n\t"
       "movl (%1), %0"
       : "=r" (a)
       : "r" (&b), "t" (y)
       : "st");
  return a;
}

int
main ()
{
  int t = -10;

  if (test (t) != t)
    __builtin_abort ();
  return 0;
}
--cut here--

BTW: It looks to me you also don't need all-memory clobber here.

This part is OK, with a testcase you provided it borders on obvious.
However, you will need rtl-optimization approval for the other
problem.

Uros.
diff mbox

Patch

Index: gcc/reg-stack.c
===================================================================
--- gcc/reg-stack.c	(revision 231598)
+++ gcc/reg-stack.c	(working copy)
@@ -97,6 +97,9 @@ 
 	All implicitly popped input regs must be closer to the top of
 	the reg-stack than any input that is not implicitly popped.
 
+	All explicitly referenced input operands may not "skip" a reg.
+	Otherwise we can have holes in the stack.
+
    3. It is possible that if an input dies in an insn, reload might
       use the input reg for an output reload.  Consider this example:
 
@@ -461,6 +464,7 @@  check_asm_stack_operands (rtx_insn *insn)
 
   char reg_used_as_output[FIRST_PSEUDO_REGISTER];
   char implicitly_dies[FIRST_PSEUDO_REGISTER];
+  char explicitly_used[FIRST_PSEUDO_REGISTER];
 
   rtx *clobber_reg = 0;
   int n_inputs, n_outputs;
@@ -568,6 +572,7 @@  check_asm_stack_operands (rtx_insn *insn)
      popped.  */
 
   memset (implicitly_dies, 0, sizeof (implicitly_dies));
+  memset (explicitly_used, 0, sizeof (explicitly_used));
   for (i = n_outputs; i < n_outputs + n_inputs; i++)
     if (STACK_REG_P (recog_data.operand[i]))
       {
@@ -581,6 +586,8 @@  check_asm_stack_operands (rtx_insn *insn)
 
 	if (j < n_clobbers || op_alt[i].matches >= 0)
 	  implicitly_dies[REGNO (recog_data.operand[i])] = 1;
+	else if (reg_class_size[(int) op_alt[i].cl] == 1)
+	  explicitly_used[REGNO (recog_data.operand[i])] = 1;
       }
 
   /* Search for first non-popped reg.  */
@@ -600,6 +607,23 @@  check_asm_stack_operands (rtx_insn *insn)
       malformed_asm = 1;
     }
 
+  /* Search for first not-explicitly used reg.  */
+  for (i = FIRST_STACK_REG; i < LAST_STACK_REG + 1; i++)
+    if (! implicitly_dies[i] && ! explicitly_used[i])
+      break;
+
+  /* If there are any other explicitly used regs, that's an error.  */
+  for (; i < LAST_STACK_REG + 1; i++)
+    if (explicitly_used[i])
+      break;
+
+  if (i != LAST_STACK_REG + 1)
+    {
+      error_for_asm (insn,
+		     "explicitly used regs must be grouped at top of stack");
+      malformed_asm = 1;
+    }
+
   /* Enforce rule #3: If any input operand uses the "f" constraint, all
      output constraints must use the "&" earlyclobber.
 
@@ -607,7 +631,7 @@  check_asm_stack_operands (rtx_insn *insn)
      record any earlyclobber.  */
 
   for (i = n_outputs; i < n_outputs + n_inputs; i++)
-    if (op_alt[i].matches == -1)
+    if (op_alt[i].matches == -1 && STACK_REG_P (recog_data.operand[i]))
       {
 	int j;
 
Index: gcc/testsuite/gcc.target/i386/pr68843-1.c
===================================================================
--- gcc/testsuite/gcc.target/i386/pr68843-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/i386/pr68843-1.c	(working copy)
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+double
+test ()
+{
+  double x = 1.0;
+  asm ("fld %1" /* { dg-error "explicitly used regs must be grouped at top of stack" } */
+       : "=&t" (x)
+       : "u" (x));
+  return x;
+}
Index: gcc/testsuite/gcc.target/i386/pr68843-2.c
===================================================================
--- gcc/testsuite/gcc.target/i386/pr68843-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/i386/pr68843-2.c	(working copy)
@@ -0,0 +1,24 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+int
+test (double y)
+{
+  int a, b;
+  asm ("fistpl (%1)\n\t"
+       "movl (%1), %0"
+       : "=r" (a)
+       : "r" (&b), "t" (y)
+       : "st", "memory");
+  return a;
+}
+
+int 
+main ()
+{
+  int t;
+  for (t = -10; t < 10; t++)
+    if (test ((double)t) != t)
+      __builtin_abort ();  
+  return 0;
+}