[PATCH][PR33133]: Fix bug in ia64 speculation support

[PATCH][PR33133]: Fix bug in ia64 speculation support

[Maxim Kuvyrkov]

[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]

Hi!

This patch fixes PR33133.

The scenario triggering the bug is the following:

1. Load instruction (ld1) with one last unresolved dependency on store 
(st) is converted to data speculative load and added to the ready list.

2. Store (st), in its turn, has a one last unresolved dependency on the 
bookkeeping copy of speculatively scheduled load (ld0).

3. Store (st) is speculative scheduled and a bookkeeping copy of it 
created in the recovery block for load (ld0).

4. All forward dependencies of store (including ld1) are made 
be_in_speculative to allow consumers of store (st) to be speculatively 
scheduled.

5. Because load (ld1) is trap_risky, its be_in_speculative dependency is 
forced to become hard.  Consequently, load (ld1) now has a hard 
dependency and is no longer ready for scheduling, which triggers a check 
in haifa-sched.c: try_ready () fail.


The proposed fix is to check if be_in_speculative dependency can be 
added to the insn before trying to do that.

OK for trunk?

Bootstrapped and regtested on ia64-linux-gnu (with c,c++ and fortran).


Thanks,

Maxim

[-- Attachment #2: pr33133.ChangeLog --]
[-- Type: text/plain, Size: 324 bytes --]

2007-09-21  Maxim Kuvyrkov  <maxim@codesourcery.com>

	PR target/33133
	
	* haifa-sched.c (process_insn_forw_deps_be_in_spec): Check if
	speculation type of insn can be changed before trying to do that.

2007-09-21  Maxim Kuvyrkov  <maxim@codesourcery.com>

	PR target/33133

	* gcc.c-torture/compile/pr33133.c: New test.
	

[-- Attachment #3: pr33133.patch --]
[-- Type: text/plain, Size: 3451 bytes --]

Index: haifa-sched.c
===================================================================
--- haifa-sched.c	(revision 128598)
+++ haifa-sched.c	(working copy)
@@ -3292,8 +3292,17 @@ process_insn_forw_deps_be_in_spec (rtx i
 		     due to backend decision.  Hence we can't let the
 		     probability of the speculative dep to decrease.  */
 		  dep_weak (ds) <= dep_weak (fs))
-		/* Transform it to be in speculative.  */
-		ds = (ds & ~BEGIN_SPEC) | fs;
+		{
+		  ds_t new_ds;
+
+		  new_ds = (ds & ~BEGIN_SPEC) | fs;
+		  
+		  if (/* consumer can 'be in speculative'.  */
+		      sched_insn_is_legitimate_for_speculation_p (consumer,
+								  new_ds))
+		    /* Transform it to be in speculative.  */
+		    ds = new_ds;
+		}
 	    }
 	  else
 	    /* Mark the dep as 'be in speculative'.  */
Index: testsuite/gcc.c-torture/compile/pr33133.c
===================================================================
--- testsuite/gcc.c-torture/compile/pr33133.c	(revision 0)
+++ testsuite/gcc.c-torture/compile/pr33133.c	(revision 0)
@@ -0,0 +1,47 @@
+static char newshuffle[256 + 16] = {
+  0x0f, 0x08, 0x05, 0x07, 0x0c, 0x02, 0x0e, 0x09, 0x00, 0x01, 0x06, 0x0d,
+    0x03, 0x04, 0x0b, 0x0a, 0x02, 0x0c, 0x0e, 0x06, 0x0f, 0x00, 0x01, 0x08,
+    0x0d, 0x03, 0x0a, 0x04, 0x09, 0x0b, 0x05, 0x07, 0x05, 0x02, 0x09, 0x0f,
+    0x0c, 0x04, 0x0d, 0x00, 0x0e, 0x0a, 0x06, 0x08, 0x0b, 0x01, 0x03, 0x07,
+    0x0f, 0x0d, 0x02, 0x06, 0x07, 0x08, 0x05, 0x09, 0x00, 0x04, 0x0c, 0x03,
+    0x01, 0x0a, 0x0b, 0x0e, 0x05, 0x0e, 0x02, 0x0b, 0x0d, 0x0a, 0x07, 0x00,
+    0x08, 0x06, 0x04, 0x01, 0x0f, 0x0c, 0x03, 0x09, 0x08, 0x02, 0x0f, 0x0a,
+    0x05, 0x09, 0x06, 0x0c, 0x00, 0x0b, 0x01, 0x0d, 0x07, 0x03, 0x04, 0x0e,
+    0x0e, 0x08, 0x00, 0x09, 0x04, 0x0b, 0x02, 0x07, 0x0c, 0x03, 0x0a, 0x05,
+    0x0d, 0x01, 0x06, 0x0f, 0x01, 0x04, 0x08, 0x0a, 0x0d, 0x0b, 0x07, 0x0e,
+    0x05, 0x0f, 0x03, 0x09, 0x00, 0x02, 0x06, 0x0c, 0x05, 0x03, 0x0c, 0x08,
+    0x0b, 0x02, 0x0e, 0x0a, 0x04, 0x01, 0x0d, 0x00, 0x06, 0x07, 0x0f, 0x09,
+    0x06, 0x00, 0x0b, 0x0e, 0x0d, 0x04, 0x0c, 0x0f, 0x07, 0x02, 0x08, 0x0a,
+    0x01, 0x05, 0x03, 0x09, 0x0b, 0x05, 0x0a, 0x0e, 0x0f, 0x01, 0x0c, 0x00,
+    0x06, 0x04, 0x02, 0x09, 0x03, 0x0d, 0x07, 0x08, 0x07, 0x02, 0x0a, 0x00,
+    0x0e, 0x08, 0x0f, 0x04, 0x0c, 0x0b, 0x09, 0x01, 0x05, 0x0d, 0x03, 0x06,
+    0x07, 0x04, 0x0f, 0x09, 0x05, 0x01, 0x0c, 0x0b, 0x00, 0x03, 0x08, 0x0e,
+    0x02, 0x0a, 0x06, 0x0d, 0x09, 0x04, 0x08, 0x00, 0x0a, 0x03, 0x01, 0x0c,
+    0x05, 0x0f, 0x07, 0x02, 0x0b, 0x0e, 0x06, 0x0d, 0x09, 0x05, 0x04, 0x07,
+    0x0e, 0x08, 0x03, 0x01, 0x0d, 0x0b, 0x0c, 0x02, 0x00, 0x0f, 0x06, 0x0a,
+    0x09, 0x0a, 0x0b, 0x0d, 0x05, 0x03, 0x0f, 0x00, 0x01, 0x0c, 0x08, 0x07,
+    0x06, 0x04, 0x0e, 0x02, 0x03, 0x0e, 0x0f, 0x02, 0x0d, 0x0c, 0x04, 0x05,
+    0x09, 0x06, 0x00, 0x01, 0x0b, 0x07, 0x0a, 0x08,
+};newpassencrypt (char *old, char *new, char *out)
+{
+  char *p, *bx;
+  char copy[8];
+  int i, di, ax;
+  char cl, dl, ch;
+  for (i = 0; i < 16; i++)
+    {
+      for (bx = old + 7; bx > old; bx--)
+        {
+          *bx = ((bx[-1] >> 4) & 0x0f) | ((*bx) << 4);
+        }
+      for (di = 0; di < 16; di++)
+        {
+          if (newshuffle[di + 0x100] & 1)
+            ch = ((copy[newshuffle[di + 0x100] / 2] >> 4) & 0x0f);
+          else
+            ch = copy[newshuffle[di + 0x100] / 2] & 0x0f;
+          out[di / 2] |= ((di & 1) ? ch << 4 : ch);
+        }
+      memcpy (copy, out, 8);
+    }
+}

Re: [PATCH][PR33133]: Fix bug in ia64 speculation support

[Andreas Schwab]

Maxim Kuvyrkov <maxim@codesourcery.com> writes:

> Index: testsuite/gcc.c-torture/compile/pr33133.c
> ===================================================================
> --- testsuite/gcc.c-torture/compile/pr33133.c	(revision 0)
> +++ testsuite/gcc.c-torture/compile/pr33133.c	(revision 0)
> @@ -0,0 +1,47 @@
> +static char newshuffle[256 + 16] = {
> +  0x0f, 0x08, 0x05, 0x07, 0x0c, 0x02, 0x0e, 0x09, 0x00, 0x01, 0x06, 0x0d,
> +    0x03, 0x04, 0x0b, 0x0a, 0x02, 0x0c, 0x0e, 0x06, 0x0f, 0x00, 0x01, 0x08,
> +    0x0d, 0x03, 0x0a, 0x04, 0x09, 0x0b, 0x05, 0x07, 0x05, 0x02, 0x09, 0x0f,
> +    0x0c, 0x04, 0x0d, 0x00, 0x0e, 0x0a, 0x06, 0x08, 0x0b, 0x01, 0x03, 0x07,
> +    0x0f, 0x0d, 0x02, 0x06, 0x07, 0x08, 0x05, 0x09, 0x00, 0x04, 0x0c, 0x03,
> +    0x01, 0x0a, 0x0b, 0x0e, 0x05, 0x0e, 0x02, 0x0b, 0x0d, 0x0a, 0x07, 0x00,
> +    0x08, 0x06, 0x04, 0x01, 0x0f, 0x0c, 0x03, 0x09, 0x08, 0x02, 0x0f, 0x0a,
> +    0x05, 0x09, 0x06, 0x0c, 0x00, 0x0b, 0x01, 0x0d, 0x07, 0x03, 0x04, 0x0e,
> +    0x0e, 0x08, 0x00, 0x09, 0x04, 0x0b, 0x02, 0x07, 0x0c, 0x03, 0x0a, 0x05,
> +    0x0d, 0x01, 0x06, 0x0f, 0x01, 0x04, 0x08, 0x0a, 0x0d, 0x0b, 0x07, 0x0e,
> +    0x05, 0x0f, 0x03, 0x09, 0x00, 0x02, 0x06, 0x0c, 0x05, 0x03, 0x0c, 0x08,
> +    0x0b, 0x02, 0x0e, 0x0a, 0x04, 0x01, 0x0d, 0x00, 0x06, 0x07, 0x0f, 0x09,
> +    0x06, 0x00, 0x0b, 0x0e, 0x0d, 0x04, 0x0c, 0x0f, 0x07, 0x02, 0x08, 0x0a,
> +    0x01, 0x05, 0x03, 0x09, 0x0b, 0x05, 0x0a, 0x0e, 0x0f, 0x01, 0x0c, 0x00,
> +    0x06, 0x04, 0x02, 0x09, 0x03, 0x0d, 0x07, 0x08, 0x07, 0x02, 0x0a, 0x00,
> +    0x0e, 0x08, 0x0f, 0x04, 0x0c, 0x0b, 0x09, 0x01, 0x05, 0x0d, 0x03, 0x06,
> +    0x07, 0x04, 0x0f, 0x09, 0x05, 0x01, 0x0c, 0x0b, 0x00, 0x03, 0x08, 0x0e,
> +    0x02, 0x0a, 0x06, 0x0d, 0x09, 0x04, 0x08, 0x00, 0x0a, 0x03, 0x01, 0x0c,
> +    0x05, 0x0f, 0x07, 0x02, 0x0b, 0x0e, 0x06, 0x0d, 0x09, 0x05, 0x04, 0x07,
> +    0x0e, 0x08, 0x03, 0x01, 0x0d, 0x0b, 0x0c, 0x02, 0x00, 0x0f, 0x06, 0x0a,
> +    0x09, 0x0a, 0x0b, 0x0d, 0x05, 0x03, 0x0f, 0x00, 0x01, 0x0c, 0x08, 0x07,
> +    0x06, 0x04, 0x0e, 0x02, 0x03, 0x0e, 0x0f, 0x02, 0x0d, 0x0c, 0x04, 0x05,
> +    0x09, 0x06, 0x00, 0x01, 0x0b, 0x07, 0x0a, 0x08,
> +};newpassencrypt (char *old, char *new, char *out)
> +{

This definition is lacking a return type.  This may result in a compile
time error in future versions of gcc.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

[PING][PATCH][PR33133]: Fix bug in ia64 speculation support

[Maxim Kuvyrkov]

[-- Attachment #1: Type: text/plain, Size: 335 bytes --]

Andreas Schwab wrote:
>> +newpassencrypt (char *old, char *new, char *out)
>> +{
> 
> This definition is lacking a return type.  This may result in a compile
> time error in future versions of gcc.

Hello,

I want to ping this patch fixing PR33133.
Attached is an updated version with fixed testcase.

:ADDPATCH ia64:


Thanks,

Maxim

[-- Attachment #2: pr33133.ChangeLog --]
[-- Type: text/plain, Size: 324 bytes --]

2007-09-21  Maxim Kuvyrkov  <maxim@codesourcery.com>

	PR target/33133
	
	* haifa-sched.c (process_insn_forw_deps_be_in_spec): Check if
	speculation type of insn can be changed before trying to do that.

2007-09-21  Maxim Kuvyrkov  <maxim@codesourcery.com>

	PR target/33133

	* gcc.c-torture/compile/pr33133.c: New test.
	

[-- Attachment #3: pr33133.patch --]
[-- Type: text/plain, Size: 3506 bytes --]

Index: haifa-sched.c
===================================================================
--- haifa-sched.c	(revision 128598)
+++ haifa-sched.c	(working copy)
@@ -3292,8 +3292,17 @@ process_insn_forw_deps_be_in_spec (rtx i
 		     due to backend decision.  Hence we can't let the
 		     probability of the speculative dep to decrease.  */
 		  dep_weak (ds) <= dep_weak (fs))
-		/* Transform it to be in speculative.  */
-		ds = (ds & ~BEGIN_SPEC) | fs;
+		{
+		  ds_t new_ds;
+
+		  new_ds = (ds & ~BEGIN_SPEC) | fs;
+		  
+		  if (/* consumer can 'be in speculative'.  */
+		      sched_insn_is_legitimate_for_speculation_p (consumer,
+								  new_ds))
+		    /* Transform it to be in speculative.  */
+		    ds = new_ds;
+		}
 	    }
 	  else
 	    /* Mark the dep as 'be in speculative'.  */
Index: testsuite/gcc.c-torture/compile/pr33133.c
===================================================================
--- testsuite/gcc.c-torture/compile/pr33133.c	(revision 0)
+++ testsuite/gcc.c-torture/compile/pr33133.c	(revision 0)
@@ -0,0 +1,50 @@
+static char newshuffle[256 + 16] = {
+  0x0f, 0x08, 0x05, 0x07, 0x0c, 0x02, 0x0e, 0x09, 0x00, 0x01, 0x06, 0x0d,
+    0x03, 0x04, 0x0b, 0x0a, 0x02, 0x0c, 0x0e, 0x06, 0x0f, 0x00, 0x01, 0x08,
+    0x0d, 0x03, 0x0a, 0x04, 0x09, 0x0b, 0x05, 0x07, 0x05, 0x02, 0x09, 0x0f,
+    0x0c, 0x04, 0x0d, 0x00, 0x0e, 0x0a, 0x06, 0x08, 0x0b, 0x01, 0x03, 0x07,
+    0x0f, 0x0d, 0x02, 0x06, 0x07, 0x08, 0x05, 0x09, 0x00, 0x04, 0x0c, 0x03,
+    0x01, 0x0a, 0x0b, 0x0e, 0x05, 0x0e, 0x02, 0x0b, 0x0d, 0x0a, 0x07, 0x00,
+    0x08, 0x06, 0x04, 0x01, 0x0f, 0x0c, 0x03, 0x09, 0x08, 0x02, 0x0f, 0x0a,
+    0x05, 0x09, 0x06, 0x0c, 0x00, 0x0b, 0x01, 0x0d, 0x07, 0x03, 0x04, 0x0e,
+    0x0e, 0x08, 0x00, 0x09, 0x04, 0x0b, 0x02, 0x07, 0x0c, 0x03, 0x0a, 0x05,
+    0x0d, 0x01, 0x06, 0x0f, 0x01, 0x04, 0x08, 0x0a, 0x0d, 0x0b, 0x07, 0x0e,
+    0x05, 0x0f, 0x03, 0x09, 0x00, 0x02, 0x06, 0x0c, 0x05, 0x03, 0x0c, 0x08,
+    0x0b, 0x02, 0x0e, 0x0a, 0x04, 0x01, 0x0d, 0x00, 0x06, 0x07, 0x0f, 0x09,
+    0x06, 0x00, 0x0b, 0x0e, 0x0d, 0x04, 0x0c, 0x0f, 0x07, 0x02, 0x08, 0x0a,
+    0x01, 0x05, 0x03, 0x09, 0x0b, 0x05, 0x0a, 0x0e, 0x0f, 0x01, 0x0c, 0x00,
+    0x06, 0x04, 0x02, 0x09, 0x03, 0x0d, 0x07, 0x08, 0x07, 0x02, 0x0a, 0x00,
+    0x0e, 0x08, 0x0f, 0x04, 0x0c, 0x0b, 0x09, 0x01, 0x05, 0x0d, 0x03, 0x06,
+    0x07, 0x04, 0x0f, 0x09, 0x05, 0x01, 0x0c, 0x0b, 0x00, 0x03, 0x08, 0x0e,
+    0x02, 0x0a, 0x06, 0x0d, 0x09, 0x04, 0x08, 0x00, 0x0a, 0x03, 0x01, 0x0c,
+    0x05, 0x0f, 0x07, 0x02, 0x0b, 0x0e, 0x06, 0x0d, 0x09, 0x05, 0x04, 0x07,
+    0x0e, 0x08, 0x03, 0x01, 0x0d, 0x0b, 0x0c, 0x02, 0x00, 0x0f, 0x06, 0x0a,
+    0x09, 0x0a, 0x0b, 0x0d, 0x05, 0x03, 0x0f, 0x00, 0x01, 0x0c, 0x08, 0x07,
+    0x06, 0x04, 0x0e, 0x02, 0x03, 0x0e, 0x0f, 0x02, 0x0d, 0x0c, 0x04, 0x05,
+    0x09, 0x06, 0x00, 0x01, 0x0b, 0x07, 0x0a, 0x08,
+};
+void newpassencrypt (char *, char *, char *);
+void
+newpassencrypt (char *old, char *new, char *out)
+{
+  char *p, *bx;
+  char copy[8];
+  int i, di, ax;
+  char cl, dl, ch;
+  for (i = 0; i < 16; i++)
+    {
+      for (bx = old + 7; bx > old; bx--)
+        {
+          *bx = ((bx[-1] >> 4) & 0x0f) | ((*bx) << 4);
+        }
+      for (di = 0; di < 16; di++)
+        {
+          if (newshuffle[di + 0x100] & 1)
+            ch = ((copy[newshuffle[di + 0x100] / 2] >> 4) & 0x0f);
+          else
+            ch = copy[newshuffle[di + 0x100] / 2] & 0x0f;
+          out[di / 2] |= ((di & 1) ? ch << 4 : ch);
+        }
+      memcpy (copy, out, 8);
+    }
+}

Re: [PING][PATCH][PR33133]: Fix bug in ia64 speculation support

[Richard Guenther]

On 10/14/07, Maxim Kuvyrkov <maxim@codesourcery.com> wrote:
> Andreas Schwab wrote:
> >> +newpassencrypt (char *old, char *new, char *out)
> >> +{
> >
> > This definition is lacking a return type.  This may result in a compile
> > time error in future versions of gcc.
>
> Hello,
>
> I want to ping this patch fixing PR33133.
> Attached is an updated version with fixed testcase.

This is ok.

Thanks,
Richard.

>
> 2007-09-21  Maxim Kuvyrkov  <maxim@codesourcery.com>
>
>         PR target/33133
>
>         * haifa-sched.c (process_insn_forw_deps_be_in_spec): Check if
>         speculation type of insn can be changed before trying to do that.
>
> 2007-09-21  Maxim Kuvyrkov  <maxim@codesourcery.com>
>
>         PR target/33133
>
>         * gcc.c-torture/compile/pr33133.c: New test.