[backport gcc-4.7/trunk r171592 ] From: Eric Botcazou Subject: [patch] Fix SSA corruption with SLP Date: Sun, 27 Mar 2011 15:40:44 +0200 List-Archive: 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 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? And the branches? gcc/ 2011-03-28 Eric Botcazou * 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. gcc/testsuite/ 2011-03-28 Eric Botcazou * gcc.dg/slp-1.c: New test. --- gcc-4.6.2/gcc/testsuite/gcc.dg/slp-1.c.~1~ 1970-01-01 01:00:00.000000000 +0100 +++ gcc-4.6.2/gcc/testsuite/gcc.dg/slp-1.c 2011-12-26 23:04:03.000000000 +0100 @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -ftree-vectorize -fno-vect-cost-model" } */ + +struct R { + double d1; + double d2; +}; + +struct R foo (struct R arg) +{ + struct R ret; + ret.d1 = arg.d2 * (0.0 - arg.d1); + ret.d2 = ret.d1; + return ret; +} --- gcc-4.6.2/gcc/tree-vect-data-refs.c.~1~ 2011-09-25 11:25:59.000000000 +0200 +++ gcc-4.6.2/gcc/tree-vect-data-refs.c 2011-12-26 23:04:03.000000000 +0100 @@ -2997,9 +2997,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 + 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 @@ -3027,9 +3028,9 @@ vect_create_addr_base_for_vector_ref (gi 4. Return the pointer. */ tree -vect_create_data_ref_ptr (gimple stmt, struct loop *at_loop, - tree offset, tree *initial_address, gimple *ptr_incr, - bool only_init, bool *inv_p) +vect_create_data_ref_ptr (gimple stmt, struct loop *at_loop, tree offset, + tree *initial_address, gimple_stmt_iterator *gsi, + gimple *ptr_incr, bool only_init, bool *inv_p) { tree base_name; stmt_vec_info stmt_info = vinfo_for_stmt (stmt); @@ -3055,7 +3056,6 @@ vect_create_data_ref_ptr (gimple stmt, s gimple incr; tree step; bb_vec_info bb_vinfo = STMT_VINFO_BB_VINFO (stmt_info); - gimple_stmt_iterator gsi = gsi_for_stmt (stmt); tree base; if (loop_vinfo) @@ -3200,7 +3200,7 @@ vect_create_data_ref_ptr (gimple stmt, s gcc_assert (!new_bb); } else - gsi_insert_seq_before (&gsi, new_stmt_list, GSI_SAME_STMT); + gsi_insert_seq_before (gsi, new_stmt_list, GSI_SAME_STMT); } *initial_address = new_temp; @@ -3222,7 +3222,7 @@ vect_create_data_ref_ptr (gimple stmt, s gcc_assert (!new_bb); } else - gsi_insert_before (&gsi, vec_stmt, GSI_SAME_STMT); + gsi_insert_before (gsi, vec_stmt, GSI_SAME_STMT); } else vect_ptr_init = new_temp; @@ -3747,7 +3747,7 @@ vect_setup_realignment (gimple stmt, gim gcc_assert (!compute_in_loop); vec_dest = vect_create_destination_var (scalar_dest, vectype); ptr = vect_create_data_ref_ptr (stmt, loop_for_initial_load, NULL_TREE, - &init_addr, &inc, true, &inv_p); + &init_addr, NULL, &inc, true, &inv_p); new_stmt = gimple_build_assign_with_ops (BIT_AND_EXPR, NULL_TREE, ptr, build_int_cst (TREE_TYPE (ptr), --- gcc-4.6.2/gcc/tree-vect-stmts.c.~1~ 2011-09-15 03:49:22.000000000 +0200 +++ gcc-4.6.2/gcc/tree-vect-stmts.c 2011-12-26 23:04:03.000000000 +0100 @@ -3611,7 +3611,7 @@ vectorizable_store (gimple stmt, gimple_ gcc_assert (useless_type_conversion_p (vectype, TREE_TYPE (vec_oprnd))); dataref_ptr = vect_create_data_ref_ptr (first_stmt, NULL, NULL_TREE, - &dummy, &ptr_incr, false, + &dummy, gsi, &ptr_incr, false, &inv_p); gcc_assert (bb_vinfo || !inv_p); } @@ -4138,9 +4138,8 @@ vectorizable_load (gimple stmt, gimple_s { /* 1. Create the vector pointer update chain. */ if (j == 0) - dataref_ptr = vect_create_data_ref_ptr (first_stmt, - at_loop, offset, - &dummy, &ptr_incr, false, + dataref_ptr = vect_create_data_ref_ptr (first_stmt, at_loop, offset, + &dummy, gsi, &ptr_incr, false, &inv_p); else dataref_ptr = --- gcc-4.6.2/gcc/tree-vectorizer.h.~1~ 2011-06-04 11:20:00.000000000 +0200 +++ gcc-4.6.2/gcc/tree-vectorizer.h 2011-12-26 23:04:03.000000000 +0100 @@ -830,7 +830,8 @@ extern bool vect_analyze_data_ref_access extern bool vect_prune_runtime_alias_test_list (loop_vec_info); extern bool vect_analyze_data_refs (loop_vec_info, bb_vec_info, int *); extern tree vect_create_data_ref_ptr (gimple, struct loop *, tree, tree *, - gimple *, bool, bool *); + gimple_stmt_iterator *, gimple *, + bool, bool *); extern tree bump_vector_ptr (tree, gimple, gimple_stmt_iterator *, gimple, tree); extern tree vect_create_destination_var (tree, tree); extern bool vect_strided_store_supported (tree);