diff mbox

Scheduler fix for the new predication code

Message ID 4EA935DD.2030803@codesourcery.com
State New
Headers show

Commit Message

Bernd Schmidt Oct. 27, 2011, 10:43 a.m. UTC 
When we try to predicate an insn in the scheduler, we must check whether
the condition has been overwritten. The current code for that tries to
look for an insn writing the condition register, then checking
REG_DEP_TRUE deps. It fails in a situation as follows:

[A1] branch
A1:A0 = some operation
some other operation using A0

We'd predicate the last insn, even though the condition was overwritten
by the middle one.

The patch below uses a simpler strategy that should be correct. If the
branch hasn't been scheduled, the condition register must have the right
value (since an insn that's a candidate for predication is made to
depend on the right setter). If the branch has been scheduled, and
another insn after it clobbers the condition register, we cannot do
predication.

This affects only C6X, and I've tested it with a 4.5 c6x-elf toolchain
so far. Will also give it a spin with 4.7. Ok?


Bernd
* haifa-sched.c (recompute_todo_spec): Simplify and correct the
	code checking for a clobber of a condition register when deciding
	whether to predicate.

Comments

Bernd Schmidt Nov. 8, 2011, 3:05 p.m. UTC | #1 
Ping. Ensure we don't predicate insns with the wrong condition in a
branch delay slot:

http://gcc.gnu.org/ml/gcc-patches/2011-10/msg02447.html


Bernd
Vladimir Makarov Nov. 8, 2011, 5:38 p.m. UTC | #2 
On 11/08/2011 10:05 AM, Bernd Schmidt wrote:
> Ping. Ensure we don't predicate insns with the wrong condition in a
> branch delay slot:
>
> http://gcc.gnu.org/ml/gcc-patches/2011-10/msg02447.html
>
>
Ok.  Thanks, Bernd.
diff mbox

Patch

Index: gcc/haifa-sched.c
===================================================================
--- gcc/haifa-sched.c	(revision 340434)
+++ gcc/haifa-sched.c	(working copy)
@@ -1180,33 +1180,20 @@  recompute_todo_spec (rtx next)
       regno = REGNO (XEXP (cond, 0));
 
       /* Find the last scheduled insn that modifies the condition register.
-	 If we have a true dependency on it, it sets it to the correct value,
-	 otherwise it must be a later insn scheduled in-between that clobbers
-	 the condition.  */
-      FOR_EACH_VEC_ELT_REVERSE (rtx, scheduled_insns, i, prev)
-	{
-	  sd_iterator_def sd_it;
-	  dep_t dep;
-	  HARD_REG_SET t;
-	  bool found;
-
-	  find_all_hard_reg_sets (prev, &t);
-	  if (!TEST_HARD_REG_BIT (t, regno))
-	    continue;
+	 We can stop looking once we find the insn we depend on through the
+	 REG_DEP_CONTROL; if the condition register isn't modified after it,
+	 we know that it still has the right value.  */
+      if (QUEUE_INDEX (pro) == QUEUE_SCHEDULED)
+	FOR_EACH_VEC_ELT_REVERSE (rtx, scheduled_insns, i, prev)
+	  {
+	    HARD_REG_SET t;
 
-	  found = false;
-	  FOR_EACH_DEP (next, SD_LIST_RES_BACK, sd_it, dep)
-	    {
-	      if (DEP_PRO (dep) == prev && DEP_TYPE (dep) == REG_DEP_TRUE)
-		{
-		  found = true;
-		  break;
-		}
-	    }
-	  if (!found)
-	    return HARD_DEP;
-	  break;
-	}
+	    find_all_hard_reg_sets (prev, &t);
+	    if (TEST_HARD_REG_BIT (t, regno))
+	      return HARD_DEP;
+	    if (prev == pro)
+	      break;
+	  }
       if (ORIG_PAT (next) == NULL_RTX)
 	{
 	  ORIG_PAT (next) = PATTERN (next);