Re: [patch] Fix SSA corruption with SLP

[Ira Rosen <IRAR at il dot ibm dot com>]

gcc-patches-owner@gcc.gnu.org wrote on 27/03/2011 03:40:44 PM:
>
> Hi,
>
> the attached testcase exhibits a corruption of the SSA form:
>
> t.c: In function 'foo':
> t.c:9:10: error: definition in block 2 follows the use
> for SSA_NAME: vect_p.7_12 in statement:
> # VUSE <.MEM_6(D)>
> vect_var_.8_13 = MEM[(struct R *)vect_p.7_12];
> t.c:9:10: internal compiler error: verify_ssa failed
> Please submit a full bug report,
> with preprocessed source if appropriate.
> See <http://gcc.gnu.org/bugs.html> for instructions.
>
> introduced by SLP on x86-64 at -O2 -ftree-vectorize -fno-vect-cost-model.
>
> This is a wrong statement rewriting in vectorizable_load.  Before SLP we
have:
>
>   D.2687_1 = arg.d2;
>   D.2688_2 = arg.d1;
>   D.2689_3 = 0.0 - D.2688_2;
>   D.2690_4 = D.2687_1 * D.2689_3;
>   D.2692.d1 = D.2690_4;
>
> vect_check_interleaving computes that field d1 is accessed before field
d2
> because the structure is defined as
>
> struct R {
>   double d1;
>   double d2;
> };
>
> but it's the opposite in the code.  So, in vectorizable_load,
> first_stmt is the
> load of d1 and new statements are wrongly inserted _after_ the load of
d2.
> Note that, on release branches (4.5 and 4.6 at least), you get wrong
code.
>
> Proposed fix attached.  It adds a GSI parameter to
vect_create_data_ref_ptr.
> Tested on {i586,x86_64}-suse-linux, OK for the mainline?

> Index: tree-vect-data-refs.c
> ===================================================================
> --- tree-vect-data-refs.c	(revision 171572)
> +++ tree-vect-data-refs.c	(working copy)
> @@ -2922,9 +2922,10 @@ vect_create_addr_base_for_vector_ref (gi
>     2. AT_LOOP: the loop where the vector memref is to be created.
>     3. OFFSET (optional): an offset to be added to the initial address
accessed
>          by the data-ref in STMT.
> -   4. ONLY_INIT: indicate if vp is to be updated in the loop, or remain
> +   4. BSI: location where the new stmts are to be placed if there is no
loop

GSI?

> +   5. ONLY_INIT: indicate if vp is to be updated in the loop, or remain
>          pointing to the initial address.
> -   5. TYPE: if not NULL indicates the required type of the data-ref.
> +   6. TYPE: if not NULL indicates the required type of the data-ref.
>
>     Output:
>     1. Declare a new ptr to vector_type, and have it point to the base of
the

OK for the mainline otherwise.

Thanks,
Ira


> And the branches?
>
>
> 2011-03-27  Eric Botcazou  <ebotcazou@adacore.com>
>
>    * tree-vectorizer.h (vect_create_data_ref_ptr): Adjust prototype.
>    * tree-vect-data-refs.c (vect_create_data_ref_ptr): Add GSI parameter.
>    Insert new statements at it in lieu of STMT.
>    (vect_setup_realignment): Adjust call to vect_create_data_ref_ptr.
>    * tree-vect-stmts.c (vectorizable_store): Likewise.
>    (vectorizable_load): Likewise.
>
>
> 2011-03-27  Eric Botcazou  <ebotcazou@adacore.com>
>
>    * gcc.dg/slp-1.c: New test.
>
>
> --
> Eric Botcazou
> [attachment "p.diff" deleted by Ira Rosen/Haifa/IBM] [attachment
> "t.c" deleted by Ira Rosen/Haifa/IBM]