Patchwork Fix PR46077: ICE in vectorizer

login
register
mail settings
Submitter Michael Matz
Date Nov. 19, 2010, 5:04 p.m.
Message ID <Pine.LNX.4.64.1011191748360.29956@wotan.suse.de>
Download mbox | patch
Permalink /patch/72274/
State New
Headers show

Comments

Michael Matz - Nov. 19, 2010, 5:04 p.m.
Hi,

the fix added for PR 45971 isn't effective for this testcase.  The problem 
is that we're presented with such code:

  src_1 = ...
  loop:
    src_3 = PHI<src_1, src_2>
    if (cond)
      src_4 = src_3 + 1;
    else
      src_5 = src_3 + 1;
    src_2 = PHI <src_4, src_5>
    goto loop

SCEV can determine that src_3 == {src_1, +, 1}_1 .  SCEV currently can not 
determine that src_2 == {src_1 + 1, +, 1}_1.  That is because the merge 
function doesn't notice that 'src_1 + 1' and 'src_1 + 1' are equal.  So we 
happily if-convert the code above:

 loop:
   src_3 = PHI<src_1, src_2>
   src_4 = src_3 + 1;
   src_5 = src_3 + 1;
   src_2 = cond ? src_4 : src_5

That leads to the problem underlying this bug: SCEV analysis returns
Sebastian Pop - Nov. 19, 2010, 6:51 p.m.
On Fri, Nov 19, 2010 at 11:04, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> the fix added for PR 45971 isn't effective for this testcase.  The problem
> is that we're presented with such code:
>
>  src_1 = ...
>  loop:
>    src_3 = PHI<src_1, src_2>
>    if (cond)
>      src_4 = src_3 + 1;
>    else
>      src_5 = src_3 + 1;
>    src_2 = PHI <src_4, src_5>
>    goto loop
>
> SCEV can determine that src_3 == {src_1, +, 1}_1 .  SCEV currently can not
> determine that src_2 == {src_1 + 1, +, 1}_1.  That is because the merge
> function doesn't notice that 'src_1 + 1' and 'src_1 + 1' are equal.  So we
> happily if-convert the code above:
>
>  loop:
>   src_3 = PHI<src_1, src_2>
>   src_4 = src_3 + 1;
>   src_5 = src_3 + 1;
>   src_2 = cond ? src_4 : src_5
>
> That leads to the problem underlying this bug: SCEV analysis returns
> different result before and after if-conversion: the vectorizer creates
> some new BBs, leading to different block_before_loop() values leading to
> accessing not-yet-cached chrecs for src_3, hence recomputing it, where we
> then run into the issue that scev analysis doesn't cope with the COND_EXPR
> we now have.
>
> Of course, that really is the problem: the analysis phase for
> vectorization (vect_can_advance_ivs_p) works with different data than the
> transformation phase (vect_update_ivs_after_vectorizer), leading to the
> ICE ultimately.
>
> The easiest fix is to make us not produce such situations, i.e. not
> producing COND_EXPR when the RHS was analyzable by SCEV.  For that we need
> to handle some more situations when merging chrecs, making the analysis
> strictly more powerful.
>
> This fixes the testcase.  Otherwise regstrapping is in progress on
> x86_64-linux (all langs+Ada).  Okay for trunk?
>

Ok.  Thanks for fixing this.

>
> Ciao,
> Michael.
> PS: it would probably be a good idea to handle COND_EXPR RHSs in scev
> analysis, like we handle condition phi-nodes already.

I agree.

Sebastian

Patch

different result before and after if-conversion: the vectorizer creates 
some new BBs, leading to different block_before_loop() values leading to 
accessing not-yet-cached chrecs for src_3, hence recomputing it, where we 
then run into the issue that scev analysis doesn't cope with the COND_EXPR 
we now have.

Of course, that really is the problem: the analysis phase for 
vectorization (vect_can_advance_ivs_p) works with different data than the 
transformation phase (vect_update_ivs_after_vectorizer), leading to the 
ICE ultimately.

The easiest fix is to make us not produce such situations, i.e. not 
producing COND_EXPR when the RHS was analyzable by SCEV.  For that we need 
to handle some more situations when merging chrecs, making the analysis 
strictly more powerful.

This fixes the testcase.  Otherwise regstrapping is in progress on 
x86_64-linux (all langs+Ada).  Okay for trunk?


Ciao,
Michael.
PS: it would probably be a good idea to handle COND_EXPR RHSs in scev 
analysis, like we handle condition phi-nodes already.

	PR tree-optimization/46077
	* tree-chrec.c (eq_evolutions_p): Accept some expressions.

testsuite/
	PR tree-optimization/46077
	* gcc.dg/vect/O3-pr46077.c: New testcase.

Index: tree-chrec.c
===================================================================
--- tree-chrec.c	(revision 166903)
+++ tree-chrec.c	(working copy)
@@ -1433,6 +1433,16 @@  eq_evolutions_p (const_tree chrec0, cons
       return (CHREC_VARIABLE (chrec0) == CHREC_VARIABLE (chrec1)
 	      && eq_evolutions_p (CHREC_LEFT (chrec0), CHREC_LEFT (chrec1))
 	      && eq_evolutions_p (CHREC_RIGHT (chrec0), CHREC_RIGHT (chrec1)));
+
+    case PLUS_EXPR:
+    case MULT_EXPR:
+    case MINUS_EXPR:
+    case POINTER_PLUS_EXPR:
+      return eq_evolutions_p (TREE_OPERAND (chrec0, 0),
+			      TREE_OPERAND (chrec1, 0))
+	  && eq_evolutions_p (TREE_OPERAND (chrec0, 1),
+			      TREE_OPERAND (chrec1, 1));
+
     default:
       return false;
     }
Index: testsuite/gcc.dg/vect/O3-pr46077.c
===================================================================
--- testsuite/gcc.dg/vect/O3-pr46077.c	(revision 0)
+++ testsuite/gcc.dg/vect/O3-pr46077.c	(revision 0)
@@ -0,0 +1,10 @@ 
+/* { dg-do compile } */
+/* { dg-require-effective-target vect_int } */
+
+void intf_pcmPlayEffect(int *src, int *dst, int size) {
+ int i;
+ for (i = 0; i < size; i++)
+  *dst++ = *src & 0x80 ? (*src++ & 0x7f) : -*src++;
+}
+
+/* { dg-final { cleanup-tree-dump "vect" } } */