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]

Re: [PATCH]: Fix bug preventing some LICM in PRE





> The vect.exp change is because this patch will cause vect-8[6-8].c to
fail
> due to the particular code it wants to see vectorized (k = i + n in the
> below example)
>

The problem that prevents vectorization from taking place when the improved
PRE is enabled, is the presence of an invariant PHI introduced by PRE:

<L36>:;
  # prephitmp.3_20 = PHI <k_21(17), pretmp.2_59(3)>;
  # j_3 = PHI <j_35(17), 0(3)>;
<L1>:;
  k_21 = pretmp.2_59;
  a[j_3] = k_21;
  j_35 = j_3 + 1;
  if (n_14 > j_35) goto <L36>; else goto <L3>;


When PRE is disabled, we have an invariant computation inside the loop that
we can vectorize:

<L36>:;
  # j_3 = PHI <j_35(17), 0(3)>;
<L1>:;
  k_59 = n_14 + i_28;
  a[j_3] = k_59;
  j_35 = j_3 + 1;
  if (n_14 > j_35) goto <L36>; else goto <L3>;


We have a patch that allows the vectorizer to overcome such invariant phis
- we were going to submit it to be able to vectorize testcase vect-85.c:
Before the PRE fix, PRE was able to remove redundancies in vect-85.c, while
introducing an invariant phi, thereby preventing vectorization (for now);
However PRE was not able to remove redundancies in vect-[86,87,88].c which
is why these testcases *did* get vectorized... Now (after the PRE fix), PRE
is able to remove redundancies also in vect-[86,87,88].c, so these
testcase, like vect-85.c, are now also not vectorized.

By the way, we mantioned this PRE issue in PR18181, which prompted the
introduction of these testcases.

> ....
>
> is completely loop invariant out of both loops, and the fixed PRE will
> happily move it.
>
> I've simply added -fno-tree-pre to the default flags for the vectorizer
> tests for now, since this is supposed to be testing vectorization, not
> general optimization.

Well, we want to check how the vectorizer behaves in a normal -O2
compilation (users will not often use -fno-tree-pre) so I would prefer
removing the -fno-tree-pre and xfail these testcases.

>  This will let vectorizer people test that we
> vectorize whatever they like.
>

If we keep the -fno-tree-pre in vect.exp we have to remove the xfail from
vect-85.c, because without PRE vect-85.c gets vectorized (we get an XPASS
with your patch, on i686-pc-linux-gnu and powerpc-darwin).

I prefer that we remove the -fno-tree-pre from vect.exp and xfail testcases
vect-[86.87.88].c for now, until we incorporate the patch that ignores
invariant phis in the vectorizer.

would that be OK for mainline?
tested on powerpc-darwin and i686-pc-linux-gnu

dorit


(See attached file: out)

gcc-patches-owner@gcc.gnu.org wrote on 17/01/2005 02:44:34:

> Steven Bosscher noticed a bug related to PRE where we were marking uses
in
> conditionals (and other types of don't care statements) as
> generating things in the TMP_GEN set.
> This is not correct, only DEFs go in TMP_GEN.
> This genreally showed up as our inability to perform loop invariant
motion
> on some things when there were conditionals with them as part of loop
> tests, etc.
>
> Fix + testcase attached.
>
> The vect.exp change is because this patch will cause vect-8[6-8].c to
fail
> due to the particular code it wants to see vectorized (k = i + n in the
> below example)
>
>   for (i = 0; i < n; i++)
>      {
>        for (j = 0; j < n; j++)
>          {
>            k = i + n;
>            a[j] = k;
>          }
>        b[i] = k;
>      }
>
> is completely loop invariant out of both loops, and the fixed PRE will
> happily move it.
>
> I've simply added -fno-tree-pre to the default flags for the vectorizer
> tests for now, since this is supposed to be testing vectorization, not
> general optimization.  This will let vectorizer people test that we
> vectorize whatever they like.
>
> It has been bootstrapped and regtestested on x86_64, ia64-linux, and
> i686-pc-linux-gnu, with no regressions (thanks to steven for submitting
> it to SuSE's autotesters).
>
> Committed to mainline.
>
> --Dan
> [attachment "cleandiff.diff" deleted by Dorit Naishlos/Haifa/IBM]

Attachment: out
Description: Binary data


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