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]

Re: [RFA:] fix PR55030, wrong code from __builtin_setjmp_receiver


On Tue, Nov 27, 2012 at 06:44:23AM -0500, Hans-Peter Nilsson wrote:
> > We apparently have a small conflict between the meaning of volatile asms with
> > operands at the source level and volatile_insn_p.  However, I think that the
> > latter interpretation is the correct one: if your asm statements have effects
> > that can be described, then you should use output constraints instead of
> > volatile; otherwise, you should use volatile and the output constraints have
> > essentially no meaning.

I strongly disagree with this.  Outputs and clobbers have significant
meaning even on volatile asms, asm volatile doesn't mean all registers and
all memory are supposed to be considered clobbered.  For memory you have
"memory" clobber for that, for registers it is users responsibility to
describe exactly what changes, either in clobbers or in outputs.
The difference between non-volatile and volatile asm is, as the
documentation states:

The `volatile' keyword indicates that the instruction has important
side-effects.  GCC will not delete a volatile `asm' if it is reachable.

Volatile asm acts as an optimization barrier to some extent, but it really
can't modify registers or memory that aren't described as modified in the
asm pattern.  The important side-effects are of some other kind than
modifying registers or memory visible from the current function.
Ditto for UNSPEC_VOLATILE.

So, at least from var-tracking POV which doesn't attempt to perform any
optimizations across any insn, just tries to track where values live, IMHO a
volatile asm acts exactly the same as non-volatile, that is why I'm testing
following patch right now.

But the question is also what effects your patch can have e.g. on RTL DSE.

2012-11-26  Jakub Jelinek  <jakub@redhat.com>

	PR debug/36728
	PR debug/55467
	* cselib.c (cselib_process_insn): If cselib_preserve_constants,
	don't reset table and exit early on volatile insns and setjmp.
	Reset table afterwards on setjmp.

	* gcc.dg/guality/pr36728-1.c: Include "../nop.h", make sure the asm
	are non-empty and add dependency between the first and second asm.
	* gcc.dg/guality/pr36728-2.c: Likewise.
	* gcc.dg/guality/pr36728-3.c: New test.
	* gcc.dg/guality/pr36728-4.c: New test.

--- gcc/cselib.c.jj	2012-11-26 10:14:26.000000000 +0100
+++ gcc/cselib.c	2012-11-26 20:01:10.488304558 +0100
@@ -2626,11 +2626,12 @@ cselib_process_insn (rtx insn)
   cselib_current_insn = insn;
 
   /* Forget everything at a CODE_LABEL, a volatile insn, or a setjmp.  */
-  if (LABEL_P (insn)
-      || (CALL_P (insn)
-	  && find_reg_note (insn, REG_SETJMP, NULL))
-      || (NONJUMP_INSN_P (insn)
-	  && volatile_insn_p (PATTERN (insn))))
+  if ((LABEL_P (insn)
+       || (CALL_P (insn)
+	   && find_reg_note (insn, REG_SETJMP, NULL))
+       || (NONJUMP_INSN_P (insn)
+	   && volatile_insn_p (PATTERN (insn))))
+      && !cselib_preserve_constants)
     {
       cselib_reset_table (next_uid);
       cselib_current_insn = NULL_RTX;
@@ -2668,9 +2669,18 @@ cselib_process_insn (rtx insn)
   /* Look for any CLOBBERs in CALL_INSN_FUNCTION_USAGE, but only
      after we have processed the insn.  */
   if (CALL_P (insn))
-    for (x = CALL_INSN_FUNCTION_USAGE (insn); x; x = XEXP (x, 1))
-      if (GET_CODE (XEXP (x, 0)) == CLOBBER)
-	cselib_invalidate_rtx (XEXP (XEXP (x, 0), 0));
+    {
+      for (x = CALL_INSN_FUNCTION_USAGE (insn); x; x = XEXP (x, 1))
+	if (GET_CODE (XEXP (x, 0)) == CLOBBER)
+	  cselib_invalidate_rtx (XEXP (XEXP (x, 0), 0));
+      /* Flush evertything on setjmp.  */
+      if (cselib_preserve_constants
+	  && find_reg_note (insn, REG_SETJMP, NULL))
+	{
+	  cselib_preserve_only_values ();
+	  cselib_reset_table (next_uid);
+	}
+    }
 
   /* On setter of the hard frame pointer if frame_pointer_needed,
      invalidate stack_pointer_rtx, so that sp and {,h}fp based
--- gcc/testsuite/gcc.dg/guality/pr36728-1.c.jj	2012-11-26 10:14:25.000000000 +0100
+++ gcc/testsuite/gcc.dg/guality/pr36728-1.c	2012-11-27 10:01:14.517080157 +0100
@@ -1,7 +1,11 @@
 /* PR debug/36728 */
 /* { dg-do run } */
 /* { dg-options "-g" } */
-int a;
+
+#include "../nop.h"
+
+int a, b;
+
 int __attribute__((noinline))
 foo (int arg1, int arg2, int arg3, int arg4, int arg5, int arg6, int arg7)
 {
@@ -9,9 +13,9 @@ foo (int arg1, int arg2, int arg3, int a
   int __attribute__ ((aligned(32))) y;
 
   y = 2;
-  asm ("" : "=m" (y) : "m" (y));
+  asm (NOP : "=m" (y), "=m" (b) : "m" (y));
   x[0] = 25;
-  asm ("" : "=m" (x[0]), "=m" (a) : "m" (x[0]));
+  asm (NOP : "=m" (x[0]), "=m" (a) : "m" (x[0]), "m" (b));
   return y;
 }
 
@@ -21,23 +25,23 @@ foo (int arg1, int arg2, int arg3, int a
    and arg2.  So it is expected that these values are unavailable in
    some of these tests.  */
 
-/* { dg-final { gdb-test 12 "arg1" "1" { target { ! "s390*-*-*" } } } }*/
-/* { dg-final { gdb-test 12 "arg2" "2" { target { ! "s390*-*-*" } } } }*/
-/* { dg-final { gdb-test 12 "arg3" "3" } } */
-/* { dg-final { gdb-test 12 "arg4" "4" } } */
-/* { dg-final { gdb-test 12 "arg5" "5" } } */
-/* { dg-final { gdb-test 12 "arg6" "6" } } */
-/* { dg-final { gdb-test 12 "arg7" "30" } } */
-/* { dg-final { gdb-test 12 "y" "2" } } */
-/* { dg-final { gdb-test 14 "arg1" "1" { target { ! "s390*-*-*" } } } }*/
-/* { dg-final { gdb-test 14 "arg2" "2" { target { ! "s390*-*-*" } } } }*/
-/* { dg-final { gdb-test 14 "arg3" "3" } } */
-/* { dg-final { gdb-test 14 "arg4" "4" } } */
-/* { dg-final { gdb-test 14 "arg5" "5" } } */
-/* { dg-final { gdb-test 14 "arg6" "6" } } */
-/* { dg-final { gdb-test 14 "arg7" "30" } } */
-/* { dg-final { gdb-test 14 "*x" "(char) 25" } } */
-/* { dg-final { gdb-test 14 "y" "2" } } */
+/* { dg-final { gdb-test 16 "arg1" "1" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 16 "arg2" "2" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 16 "arg3" "3" } } */
+/* { dg-final { gdb-test 16 "arg4" "4" } } */
+/* { dg-final { gdb-test 16 "arg5" "5" } } */
+/* { dg-final { gdb-test 16 "arg6" "6" } } */
+/* { dg-final { gdb-test 16 "arg7" "30" } } */
+/* { dg-final { gdb-test 16 "y" "2" } } */
+/* { dg-final { gdb-test 18 "arg1" "1" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 18 "arg2" "2" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 18 "arg3" "3" } } */
+/* { dg-final { gdb-test 18 "arg4" "4" } } */
+/* { dg-final { gdb-test 18 "arg5" "5" } } */
+/* { dg-final { gdb-test 18 "arg6" "6" } } */
+/* { dg-final { gdb-test 18 "arg7" "30" } } */
+/* { dg-final { gdb-test 18 "*x" "(char) 25" } } */
+/* { dg-final { gdb-test 18 "y" "2" } } */
 
 int
 main ()
--- gcc/testsuite/gcc.dg/guality/pr36728-2.c.jj	2012-11-26 10:14:25.000000000 +0100
+++ gcc/testsuite/gcc.dg/guality/pr36728-2.c	2012-11-27 10:00:46.237254022 +0100
@@ -1,7 +1,11 @@
 /* PR debug/36728 */
 /* { dg-do run } */
 /* { dg-options "-g" } */
-int a;
+
+#include "../nop.h"
+
+int a, b;
+
 int __attribute__((noinline))
 foo (int arg1, int arg2, int arg3, int arg4, int arg5, int arg6, int arg7)
 {
@@ -9,9 +13,9 @@ foo (int arg1, int arg2, int arg3, int a
   int __attribute__ ((aligned(32))) y;
 
   y = 2;
-  asm ("" : "=m" (y) : "m" (y));
+  asm (NOP : "=m" (y), "=m" (b) : "m" (y));
   x[0] = 25;
-  asm ("" : "=m" (x[0]), "=m" (a) : "m" (x[0]));
+  asm (NOP : "=m" (x[0]), "=m" (a) : "m" (x[0]), "m" (b));
   return y;
 }
 
@@ -21,23 +25,23 @@ foo (int arg1, int arg2, int arg3, int a
    and arg2.  So it is expected that these values are unavailable in
    some of these tests.  */
 
-/* { dg-final { gdb-test 12 "arg1" "1" { target { ! "s390*-*-*" } } } } */
-/* { dg-final { gdb-test 12 "arg2" "2" { target { ! "s390*-*-*" } } } } */
-/* { dg-final { gdb-test 12 "arg3" "3" } } */
-/* { dg-final { gdb-test 12 "arg4" "4" } } */
-/* { dg-final { gdb-test 12 "arg5" "5" } } */
-/* { dg-final { gdb-test 12 "arg6" "6" } } */
-/* { dg-final { gdb-test 12 "arg7" "30" } } */
-/* { dg-final { gdb-test 12 "y" "2" } } */
-/* { dg-final { gdb-test 14 "arg1" "1" { target { ! "s390*-*-*" } } } } */
-/* { dg-final { gdb-test 14 "arg2" "2" { target { ! "s390*-*-*" } } } } */
-/* { dg-final { gdb-test 14 "arg3" "3" } } */
-/* { dg-final { gdb-test 14 "arg4" "4" } } */
-/* { dg-final { gdb-test 14 "arg5" "5" } } */
-/* { dg-final { gdb-test 14 "arg6" "6" } } */
-/* { dg-final { gdb-test 14 "arg7" "30" } } */
-/* { dg-final { gdb-test 14 "*x" "(char) 25" } } */
-/* { dg-final { gdb-test 14 "y" "2" } } */
+/* { dg-final { gdb-test 16 "arg1" "1" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 16 "arg2" "2" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 16 "arg3" "3" } } */
+/* { dg-final { gdb-test 16 "arg4" "4" } } */
+/* { dg-final { gdb-test 16 "arg5" "5" } } */
+/* { dg-final { gdb-test 16 "arg6" "6" } } */
+/* { dg-final { gdb-test 16 "arg7" "30" } } */
+/* { dg-final { gdb-test 16 "y" "2" } } */
+/* { dg-final { gdb-test 18 "arg1" "1" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 18 "arg2" "2" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 18 "arg3" "3" } } */
+/* { dg-final { gdb-test 18 "arg4" "4" } } */
+/* { dg-final { gdb-test 18 "arg5" "5" } } */
+/* { dg-final { gdb-test 18 "arg6" "6" } } */
+/* { dg-final { gdb-test 18 "arg7" "30" } } */
+/* { dg-final { gdb-test 18 "*x" "(char) 25" } } */
+/* { dg-final { gdb-test 18 "y" "2" } } */
 
 int
 main ()
--- gcc/testsuite/gcc.dg/guality/pr36728-3.c.jj	2012-11-26 18:58:40.896599886 +0100
+++ gcc/testsuite/gcc.dg/guality/pr36728-3.c	2012-11-27 10:01:45.455892012 +0100
@@ -0,0 +1,51 @@
+/* PR debug/36728 */
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+#include "../nop.h"
+
+int __attribute__((noinline))
+foo (int arg1, int arg2, int arg3, int arg4, int arg5, int arg6, int arg7)
+{
+  char *x = __builtin_alloca (arg7);
+  int __attribute__ ((aligned(32))) y;
+
+  y = 2;
+  asm (NOP : "=m" (y) : "m" (y));
+  x[0] = 25;
+  asm volatile (NOP : "=m" (x[0]) : "m" (x[0]));
+  return y;
+}
+
+/* On s390(x) r2 and r3 are (depending on the optimization level) used
+   when adjusting the addresses in order to meet the alignment
+   requirements above.  They usually hold the function arguments arg1
+   and arg2.  So it is expected that these values are unavailable in
+   some of these tests.  */
+
+/* { dg-final { gdb-test 14 "arg1" "1" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 14 "arg2" "2" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 14 "arg3" "3" } } */
+/* { dg-final { gdb-test 14 "arg4" "4" } } */
+/* { dg-final { gdb-test 14 "arg5" "5" } } */
+/* { dg-final { gdb-test 14 "arg6" "6" } } */
+/* { dg-final { gdb-test 14 "arg7" "30" } } */
+/* { dg-final { gdb-test 14 "y" "2" } } */
+/* { dg-final { gdb-test 16 "arg1" "1" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 16 "arg2" "2" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 16 "arg3" "3" } } */
+/* { dg-final { gdb-test 16 "arg4" "4" } } */
+/* { dg-final { gdb-test 16 "arg5" "5" } } */
+/* { dg-final { gdb-test 16 "arg6" "6" } } */
+/* { dg-final { gdb-test 16 "arg7" "30" } } */
+/* { dg-final { gdb-test 16 "*x" "(char) 25" } } */
+/* { dg-final { gdb-test 16 "y" "2" } } */
+
+int
+main ()
+{
+  int l = 0;
+  asm volatile ("" : "=r" (l) : "0" (l));
+  foo (l + 1, l + 2, l + 3, l + 4, l + 5, l + 6, l + 30);
+  return 0;
+}
--- gcc/testsuite/gcc.dg/guality/pr36728-4.c.jj	2012-11-26 18:58:44.624580656 +0100
+++ gcc/testsuite/gcc.dg/guality/pr36728-4.c	2012-11-26 14:56:12.000000000 +0100
@@ -0,0 +1,51 @@
+/* PR debug/36728 */
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+#include "../nop.h"
+
+int __attribute__((noinline))
+foo (int arg1, int arg2, int arg3, int arg4, int arg5, int arg6, int arg7)
+{
+  char x[30];
+  int __attribute__ ((aligned(32))) y;
+
+  y = 2;
+  asm (NOP : "=m" (y) : "m" (y));
+  x[0] = 25;
+  asm volatile (NOP : "=m" (x[0]) : "m" (x[0]));
+  return y;
+}
+
+/* On s390(x) r2 and r3 are (depending on the optimization level) used
+   when adjusting the addresses in order to meet the alignment
+   requirements above.  They usually hold the function arguments arg1
+   and arg2.  So it is expected that these values are unavailable in
+   some of these tests.  */
+
+/* { dg-final { gdb-test 14 "arg1" "1" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 14 "arg2" "2" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 14 "arg3" "3" } } */
+/* { dg-final { gdb-test 14 "arg4" "4" } } */
+/* { dg-final { gdb-test 14 "arg5" "5" } } */
+/* { dg-final { gdb-test 14 "arg6" "6" } } */
+/* { dg-final { gdb-test 14 "arg7" "30" } } */
+/* { dg-final { gdb-test 14 "y" "2" } } */
+/* { dg-final { gdb-test 16 "arg1" "1" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 16 "arg2" "2" { target { ! "s390*-*-*" } } } } */
+/* { dg-final { gdb-test 16 "arg3" "3" } } */
+/* { dg-final { gdb-test 16 "arg4" "4" } } */
+/* { dg-final { gdb-test 16 "arg5" "5" } } */
+/* { dg-final { gdb-test 16 "arg6" "6" } } */
+/* { dg-final { gdb-test 16 "arg7" "30" } } */
+/* { dg-final { gdb-test 16 "*x" "(char) 25" } } */
+/* { dg-final { gdb-test 16 "y" "2" } } */
+
+int
+main ()
+{
+  int l = 0;
+  asm volatile ("" : "=r" (l) : "0" (l));
+  foo (l + 1, l + 2, l + 3, l + 4, l + 5, l + 6, l + 30);
+  return 0;
+}


	Jakub


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