diff mbox

[2/9,doloop] Correct extracting loop exit condition

Message ID CAJnFk2da+EXCr_2ywRMGzJ9eP=i1wHaGi+4QgtR+6ieJAtDMyw@mail.gmail.com
State New
Headers show

Commit Message

Roman Zhuykov Sept. 30, 2011, 3:21 p.m. UTC 
2011/7/22 Richard Sandiford <richard.sandiford@linaro.org>:
> zhroma@ispras.ru writes:
>> This patch fixes the compiler segfault found while regtesting trunk with SMS on
>> IA64 platform.  Segfault happens on test gcc.dg/pr45259.c with -fmodulo-sched
>> enabled.  The following jump instruction is given as argument for
>> doloop_condition_get function:
>> (jump_insn 86 85 88 7 (set (pc)
>>         (reg/f:DI 403)) 339 {indirect_jump}
>>      (expr_list:REG_DEAD (reg/f:DI 403)
>>         (nil)))
>> The patch adds checking for the form of comparison instruction before
>> extracting loop exit condition.
>>
>> 2011-07-20  Roman Zhuykov  <zhroma@ispras.ru>
>>       * loop-doloop.c (doloop_condition_get): Correctly check
>>       the form of comparison instruction.
>> ---
>>  gcc/loop-doloop.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/gcc/loop-doloop.c b/gcc/loop-doloop.c
>> index f8429c4..dfc4a16 100644
>> --- a/gcc/loop-doloop.c
>> +++ b/gcc/loop-doloop.c
>> @@ -153,6 +153,8 @@ doloop_condition_get (rtx doloop_pat)
>>        else
>>          inc = PATTERN (prev_insn);
>>        /* We expect the condition to be of the form (reg != 0)  */
>> +      if (GET_CODE (cmp) != SET || GET_CODE (SET_SRC (cmp)) != IF_THEN_ELSE)
>> +     return 0;
>>        cond = XEXP (SET_SRC (cmp), 0);
>>        if (GET_CODE (cond) != NE || XEXP (cond, 1) != const0_rtx)
>>          return 0;
>
> I think it'd be better to integrate:
>
>      /* We expect the condition to be of the form (reg != 0)  */
>      cond = XEXP (SET_SRC (cmp), 0);
>      if (GET_CODE (cond) != NE || XEXP (cond, 1) != const0_rtx)
>        return 0;
>
> into:
>
>  /* We expect a GE or NE comparison with 0 or 1.  */
>  if ((GET_CODE (condition) != GE
>       && GET_CODE (condition) != NE)
>      || (XEXP (condition, 1) != const0_rtx
>          && XEXP (condition, 1) != const1_rtx))
>    return 0;
>
> The next "if" already uses "GET_CODE (pattern) != PARALLEL" as a check
> for the second and third cases.  E.g. something like:
>
>  if (GET_CODE (pattern) == PARALLEL)
>    {
>      /* We expect a GE or NE comparison with 0 or 1.  */
>      if ((GET_CODE (condition) != GE
>           && GET_CODE (condition) != NE)
>          || (XEXP (condition, 1) != const0_rtx
>              && XEXP (condition, 1) != const1_rtx))
>        return 0;
>    }
>  else
>    {
>      /* In the second and third cases, we expect the condition to
>         be of the form (reg != 0)  */
>      if (GET_CODE (condition) != NE || XEXP (condition, 1) != const0_rtx)
>        return 0;
>    }
>
> That's pre-approved (independently of the other patches) if it works.
Changed like the following. Will commit if no objections after a couple of days.

--
Roman Zhuykov
zhroma@ispras.ru

Comments

Andrey Belevantsev Feb. 10, 2012, 11:58 a.m. UTC | #1 
Hello Richard,

On 30.09.2011 19:21, Roman Zhuykov wrote:
> 2011/7/22 Richard Sandiford<richard.sandiford@linaro.org>:
>> That's pre-approved (independently of the other patches) if it works.
...
> Changed like the following. Will commit if no objections after a couple of days.

We forgot to commit this patch back in September, is it fine to commit at 
this stage?

Andrey


>
> --
> Roman Zhuykov
> zhroma@ispras.ru
Richard Sandiford Feb. 17, 2012, 9:34 a.m. UTC | #2 
Andrey Belevantsev <abel@ispras.ru> writes:
> On 30.09.2011 19:21, Roman Zhuykov wrote:
>> 2011/7/22 Richard Sandiford<richard.sandiford@linaro.org>:
>>> That's pre-approved (independently of the other patches) if it works.
> ...
>> Changed like the following. Will commit if no objections after a couple of days.
>
> We forgot to commit this patch back in September, is it fine to commit at 
> this stage?

Probably a bit late now, sorry.  (A bit like my reply.)

Richard
diff mbox

Patch

2011-09-30  Roman Zhuykov  <zhroma@ispras.ru>
	* loop-doloop.c (doloop_condition_get): Correctly check
	the form of comparison instruction.
---
 gcc/loop-doloop.c |   28 +++++++++++++++++-----------
 1 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/gcc/loop-doloop.c b/gcc/loop-doloop.c
index a7e264f..4e83649 100644
--- a/gcc/loop-doloop.c
+++ b/gcc/loop-doloop.c
@@ -113,7 +113,6 @@  doloop_condition_get (rtx doloop_pat)
 
   if (GET_CODE (pattern) != PARALLEL)
     {
-      rtx cond;
       rtx prev_insn = prev_nondebug_insn (doloop_pat);
       rtx cmp_arg1, cmp_arg2;
       rtx cmp_orig;
@@ -152,10 +151,6 @@  doloop_condition_get (rtx doloop_pat)
 	}
       else
         inc = PATTERN (prev_insn);
-      /* We expect the condition to be of the form (reg != 0)  */
-      cond = XEXP (SET_SRC (cmp), 0);
-      if (GET_CODE (cond) != NE || XEXP (cond, 1) != const0_rtx)
-        return 0;
     }
   else
     {
@@ -193,12 +188,23 @@  doloop_condition_get (rtx doloop_pat)
   /* Extract loop termination condition.  */
   condition = XEXP (SET_SRC (cmp), 0);
 
-  /* We expect a GE or NE comparison with 0 or 1.  */
-  if ((GET_CODE (condition) != GE
-       && GET_CODE (condition) != NE)
-      || (XEXP (condition, 1) != const0_rtx
-          && XEXP (condition, 1) != const1_rtx))
-    return 0;
+  if (GET_CODE (pattern) == PARALLEL)
+    {
+      /* We expect a GE or NE comparison with 0 or 1.  */
+      if ((GET_CODE (condition) != GE
+	   && GET_CODE (condition) != NE)
+	   || (XEXP (condition, 1) != const0_rtx
+	   && XEXP (condition, 1) != const1_rtx))
+        return 0;
+    }
+  else
+    {
+      /* In the second and third cases, we expect the condition
+         to be of the form (reg != 0)  */
+      if (GET_CODE (condition) != NE
+	  || XEXP (condition, 1) != const0_rtx)
+        return 0;
+    }
 
   if ((XEXP (condition, 0) == reg)
       /* For the third case:  */