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][IA64][PR35659] Fix data speculation


Hello,

Here is a simple for PR target/35659. The problem is in speculatively scheduling predicated instruction and then recovering from unsuccessful data speculation.

When speculatively executing a predicated instruction, the predicate, which is calculated from wrong data provided by data-speculative load, is true. Then, when recovering from unsuccessful data-speculation, the predicate is calculated to be false and the predicated instruction is not executed, thus not recovering from effect caused by unsuccessful data-speculation.

The patch forbids predicated instructions to be scheduled as part of data speculation. This is a conservative fix, as it is legal to speculatively schedule predicated instructions whose arguments, but not predicate, are calculated from speculatively loaded data, but, at the moment, we don't track dependencies at that level of precision to say what part of consumer depends on a particular producer.

I didn't manage to extract a single-file testcase from the bug report, so there's none attached to the patch, sorry.

I don't have access to ia64 box at the moment, so I only checked it on a cross-compiler. The fix is fairly simple and only makes scheduler to use data speculation less frequently. Still, does anyone have a possibility to run tests on a cumulative patch of this and 2 other fixes for ia64 speculation I posted earlier today?

OK for trunk and 4.3 branch?


Thanks,


Maxim
2008-08-02  Maxim Kuvyrkov  <maxim@codesourcery.com>

	PR target/35659
	* haifa-sched.c (sched_insn_is_legitimate_for_speculation_p): Move ...
	* sched-deps.c (sched_insn_is_legitimate_for_speculation_p): ... here.
	Don't allow predicated instructions for data speculation.
	* sched-int.h (sched_insn_is_legitimate_for_speculation_p): Move
	declaration.
Index: gcc/haifa-sched.c
===================================================================
--- gcc/haifa-sched.c	(revision 138551)
+++ gcc/haifa-sched.c	(working copy)
@@ -4015,32 +4022,6 @@ change_pattern (rtx insn, rtx new_pat)
   dfa_clear_single_insn_cache (insn);
 }
 
-/* Return true if INSN can potentially be speculated with type DS.  */
-bool
-sched_insn_is_legitimate_for_speculation_p (const_rtx insn, ds_t ds)
-{
-  if (HAS_INTERNAL_DEP (insn))
-    return false;
-
-  if (!NONJUMP_INSN_P (insn))
-    return false;
-
-  if (SCHED_GROUP_P (insn))
-    return false;
-
-  if (IS_SPECULATION_CHECK_P (insn))
-    return false;
-
-  if (side_effects_p (PATTERN (insn)))
-    return false;
-
-  if ((ds & BE_IN_SPEC)
-      && may_trap_p (PATTERN (insn)))
-    return false;
-
-  return true;
-}
-
 /* -1 - can't speculate,
    0 - for speculation with REQUEST mode it is OK to use
    current instruction pattern,
Index: gcc/sched-deps.c
===================================================================
--- gcc/sched-deps.c	(revision 138551)
+++ gcc/sched-deps.c	(working copy)
@@ -536,6 +536,46 @@ sched_insns_conditions_mutex_p (const_rt
 }
 
 
+/* Return true if INSN can potentially be speculated with type DS.  */
+bool
+sched_insn_is_legitimate_for_speculation_p (const_rtx insn, ds_t ds)
+{
+  if (HAS_INTERNAL_DEP (insn))
+    return false;
+
+  if (!NONJUMP_INSN_P (insn))
+    return false;
+
+  if (SCHED_GROUP_P (insn))
+    return false;
+
+  if (IS_SPECULATION_CHECK_P (insn))
+    return false;
+
+  if (side_effects_p (PATTERN (insn)))
+    return false;
+
+  if (ds & BE_IN_SPEC)
+    /* The following instructions, which depend on a speculatively scheduled
+       instruction, cannot be speculatively scheduled along.  */
+    {
+      if (may_trap_p (PATTERN (insn)))
+	/* If instruction might trap, it cannot be speculatively scheduled.
+	   For control speculation it's obvious why and for data speculation
+	   it's because the insn might get wrong input if speculation
+	   wasn't successful.  */
+	return false;
+
+      if ((ds & BE_IN_DATA)
+	  && sched_get_condition (insn) != NULL_RTX)
+	/* If this is a predicated instruction, then it cannot be
+	   speculatively scheduled.  See PR35659.  */
+	return false;
+    }
+
+  return true;
+}
+
 /* Initialize LIST_PTR to point to one of the lists present in TYPES_PTR,
    initialize RESOLVED_P_PTR with true if that list consists of resolved deps,
    and remove the type of returned [through LIST_PTR] list from TYPES_PTR.
Index: gcc/sched-int.h
===================================================================
--- gcc/sched-int.h	(revision 138551)
+++ gcc/sched-int.h	(working copy)
@@ -811,6 +811,7 @@ enum INSN_TRAP_CLASS
 
 /* Functions in sched-deps.c.  */
 extern bool sched_insns_conditions_mutex_p (const_rtx, const_rtx);
+extern bool sched_insn_is_legitimate_for_speculation_p (const_rtx, ds_t);
 extern void add_dependence (rtx, rtx, enum reg_note);
 extern void sched_analyze (struct deps *, rtx, rtx);
 extern bool deps_pools_are_empty_p (void);
@@ -844,7 +845,6 @@ extern void sched_finish (void);
 
 extern int try_ready (rtx);
 extern void * xrecalloc (void *, size_t, size_t, size_t);
-extern bool sched_insn_is_legitimate_for_speculation_p (const_rtx, ds_t);
 extern void unlink_bb_notes (basic_block, basic_block);
 extern void add_block (basic_block, basic_block);
 extern rtx bb_note (basic_block);

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