Re: [patch] Cleanup and improvement of if-conversion for vectorization

[Richard Guenther <richard dot guenther at gmail dot com>]

On Mon, May 24, 2010 at 8:29 PM, Sebastian Pop <sebpop@gmail.com> wrote:
> Hi,
>
> The attached patch-set cleans the code of the current if-conversion
> pass, separates the analysis of the basic block predicates from the
> transformation, and improves the if-conversion by transforming loops
> containing conditions with memory references. ?Conditional writes to
> memory are handled as non conditional writes by reading and writing
> back the same value, like this:
>
> +/* Predicate each write to memory in LOOP.
> +
> + ? Replace a statement "A[i] = foo" with "A[i] = cond ? foo : A[i]"
> + ? with the condition COND determined from the ->aux field of the
> + ? basic block containing the statement. ?*/

Without looking at any of the patches (will do so tomorrow) this
is store-speculation which may introduce data races.  This is
not generally ok (and we removed such cases in the past).
At least not if you do not make sure the conditional store is
expanded to something that does not do store-speculation.

Richard.

> With this patch set, I am XFAIL-ing vect-ifcvt-18.c that I am
> considering unsafe for the if-conversion as currently implemented in
> trunk. ?I am XFAIL-ing it until I get a patch that checks that a
> statically allocated data reference cannot trap in a loop accessed
> niter times. ?vect-ifcvt-18.c looks like the testcase of
> http://gcc.gnu.org/PR43423
>
> __attribute__ ((noinline)) void
> foo (int mid, int n)
> {
> ?int i;
>
> ?for (i = 0; i < n; i++)
> ? ?if (i < mid)
> ? ? ?A[i] = A[i] + B[i];
> ? ?else
> ? ? ?A[i] = A[i] + C[i];
> }
>
> After if-conversion the accesses to B and C would be executed for
> every iteration, and without proving that B and C have "n" elements,
> the if-converted code could trap.
>
> The patch-set passes test and bootstrap with BOOT_CFLAGS="-g -O3".
> Ok for trunk?
>
> Thanks,
> Sebastian
>