This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: fix PR46029: reimplement if conversion of loads and stores
- From: Jeff Law <law at redhat dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: Alan Lawrence <alan dot lawrence at arm dot com>, Abe Skolnik <a dot skolnik at samsung dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, "sebpop at gmail dot com" <sebpop at gmail dot com>
- Date: Mon, 06 Jul 2015 14:46:39 -0600
- Subject: Re: fix PR46029: reimplement if conversion of loads and stores
- Authentication-results: sourceware.org; auth=none
- References: <20150612205047 dot GA27819 at cc00 dot spa dot sarc dot sas> <55883757 dot 8070105 at arm dot com> <558AE47E dot 7050408 at redhat dot com> <CAFiYyc3zhrwcXn2=PgJua4=k6XtJsan_WGAtvbw7fz7kSAkSRg at mail dot gmail dot com>
On 06/25/2015 03:43 AM, Richard Biener wrote:
Yes, and if you just look at scalar code then with the rewrite we can
enable store if-conversion unconditionally.
_But_
when the new scheme triggers vectorization cannot succeed on the
result as we get
if (cond)
*p = val;
if-converted to
tem = cond ? p : &scratch;
*tem = val;
and
if (cond)
val = *p;
if-converted to
scatch = val;
tem = cond ? p : &scratch;
val = *tem;
and thus the store and loads appear as scather/gather ones to
the vectorizer (if-conversion could directly generate masked
load/stores of course and not use a scratch-pad at all in that case).
Right. But are we correctly handling these cases in the current if
conversion code? If we're currently mis-handling them, then Abe's
changes would seem like a step forward from a correctness standpoint,
even if they take us a step backwards from a performance standpoint.
So the question is whether we get more non-vectorized if-converted
code out of this (and thus whether we want to use
--param allow-store-data-races to get the old code back which is
nicer to less capable CPUs and probably faster than using
scatter/gather or masked loads/stores).
I do think conditionalizing some of this on the allo-store-data-races
makes sense.
Note that I think scatter
support is still not implemented in the vectorizer. I also wonder
if we should really care about load data races (not sure your patch
does).
I thought the consensus was that load data races weren't important from
a correctness standpoint. Torvald would be the man to know the answer
on that one (triegel@redhat.com).
In general I like the idea.
Similarly. Just trying to find the time to actually look at it is
proving hard :(
jeff