On Mon, Jun 25, 2012 at 5:44 PM, Ulrich Weigand <uweig...@de.ibm.com> wrote:
> Richard Guenther wrote:
>
>> In this testcase the alignment of arr[i] should be irrelevant - it is
>> not part of
>> the stmts that are going to be vectorized. But of course this may be
>> simply an odering issue in how we analyze data-references / statements
>> in basic-block vectorization (thus we possibly did not yet declare the
>> arr[i] = i statement as not taking part in the vectorization).
>
> The following patch changes
> tree-vect-data-refs.c:vect_verify_datarefs_alignment
> to only take into account statements marked as "relevant".
>
> This should have no impact for loop-based vectorization, since the only caller
> (vect_enhance_data_refs_alignment) already skips irrelevant statements.
> [ As an aside, that routine should probably use STMT_VINFO_RELEVANT_P instead
> of just checking STMT_VINFO_RELEVANT as a boolean. ]
>
> However, for SLP this change in itself doesn't work, since
> vect_slp_analyze_bb_1
> calls vect_verify_datarefs_alignment *before* statements have actually been
> marked as relevant or irrelevant. Therefore, the patch also rearranges the
> sequence in vect_slp_analyze_bb_1.
>
> This in turn caused ICEs in vect_get_store_cost/vect_get_load_cost, since
> those
> now can get called with statements with unsupported alignment. There is
> already
> one caller (vect_get_data_access_cost) that checks for this case and simply
> returns "infinite" cost instead of aborting. The patch moves this behaviour
> into vect_get_store_cost/vect_get_load_cost themselves. (The particular cost
> shouldn't matter since vectorization will still be rejected in the end, it's
> just that the test now runs a bit later.)
>
> Overall, I've tested the patch with no regresions on powerpc64-linux and
> arm-linux-gnueabi. On PowerPC, it fixed the gcc.dg/vect/bb-slp-16.c test.
>
> Mikael, would you mind verifying that it fixes the problem on sparc64?
>
> OK for mainline?
Ok.
Thanks,
Richard.
> Bye,
> Ulrich
>
>
> ChangeLog:
>
> PR tree-optimization/53729
> PR tree-optimization/53636
> * tree-vect-slp.c (vect_slp_analyze_bb_1): Delay call to
> vect_verify_datarefs_alignment until after statements have
> been marked as relevant/irrelevant.
> * tree-vect-data-refs.c (vect_verify_datarefs_alignment):
> Skip irrelevant statements.
> (vect_enhance_data_refs_alignment): Use STMT_VINFO_RELEVANT_P
> instead of STMT_VINFO_RELEVANT.
> (vect_get_data_access_cost): Do not check for supportable
> alignment before calling vect_get_load_cost/vect_get_store_cost.
> * tree-vect-stmts.c (vect_get_store_cost): Do not abort when
> handling unsupported alignment.
> (vect_get_load_cost): Likewise.
>
>
> Index: gcc/tree-vect-data-refs.c
> ===================================================================
> *** gcc/tree-vect-data-refs.c (revision 188850)
> --- gcc/tree-vect-data-refs.c (working copy)
> *************** vect_verify_datarefs_alignment (loop_vec
> *** 1094,1099 ****
> --- 1094,1102 ----
> gimple stmt = DR_STMT (dr);
> stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
>
> + if (!STMT_VINFO_RELEVANT_P (stmt_info))
> + continue;
> +
> /* For interleaving, only the alignment of the first access matters.
> Skip statements marked as not vectorizable. */
> if ((STMT_VINFO_GROUPED_ACCESS (stmt_info)
> *************** vect_get_data_access_cost (struct data_r
> *** 1213,1229 ****
> loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
> int vf = LOOP_VINFO_VECT_FACTOR (loop_vinfo);
> int ncopies = vf / nunits;
> - bool supportable_dr_alignment = vect_supportable_dr_alignment (dr, true);
>
> ! if (!supportable_dr_alignment)
> ! *inside_cost = VECT_MAX_COST;
> else
> ! {
> ! if (DR_IS_READ (dr))
> ! vect_get_load_cost (dr, ncopies, true, inside_cost, outside_cost);
> ! else
> ! vect_get_store_cost (dr, ncopies, inside_cost);
> ! }
>
> if (vect_print_dump_info (REPORT_COST))
> fprintf (vect_dump, "vect_get_data_access_cost: inside_cost = %d, "
> --- 1216,1226 ----
> loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
> int vf = LOOP_VINFO_VECT_FACTOR (loop_vinfo);
> int ncopies = vf / nunits;
>
> ! if (DR_IS_READ (dr))
> ! vect_get_load_cost (dr, ncopies, true, inside_cost, outside_cost);
> else
> ! vect_get_store_cost (dr, ncopies, inside_cost);
>
> if (vect_print_dump_info (REPORT_COST))
> fprintf (vect_dump, "vect_get_data_access_cost: inside_cost = %d, "
> *************** vect_enhance_data_refs_alignment (loop_v
> *** 1537,1543 ****
> stmt = DR_STMT (dr);
> stmt_info = vinfo_for_stmt (stmt);
>
> ! if (!STMT_VINFO_RELEVANT (stmt_info))
> continue;
>
> /* For interleaving, only the alignment of the first access
> --- 1534,1540 ----
> stmt = DR_STMT (dr);
> stmt_info = vinfo_for_stmt (stmt);
>
> ! if (!STMT_VINFO_RELEVANT_P (stmt_info))
> continue;
>
> /* For interleaving, only the alignment of the first access
> Index: gcc/tree-vect-stmts.c
> ===================================================================
> *** gcc/tree-vect-stmts.c (revision 188850)
> --- gcc/tree-vect-stmts.c (working copy)
> *************** vect_get_store_cost (struct data_referen
> *** 931,936 ****
> --- 931,946 ----
> break;
> }
>
> + case dr_unaligned_unsupported:
> + {
> + *inside_cost = VECT_MAX_COST;
> +
> + if (vect_print_dump_info (REPORT_COST))
> + fprintf (vect_dump, "vect_model_store_cost: unsupported access.");
> +
> + break;
> + }
> +
> default:
> gcc_unreachable ();
> }
> *************** vect_get_load_cost (struct data_referenc
> *** 1094,1099 ****
> --- 1104,1119 ----
> break;
> }
>
> + case dr_unaligned_unsupported:
> + {
> + *inside_cost = VECT_MAX_COST;
> +
> + if (vect_print_dump_info (REPORT_COST))
> + fprintf (vect_dump, "vect_model_load_cost: unsupported access.");
> +
> + break;
> + }
> +
> default:
> gcc_unreachable ();
> }
> Index: gcc/tree-vect-slp.c
> ===================================================================
> *** gcc/tree-vect-slp.c (revision 188850)
> --- gcc/tree-vect-slp.c (working copy)
> *************** vect_slp_analyze_bb_1 (basic_block bb)
> *** 2050,2065 ****
> return NULL;
> }
>
> - if (!vect_verify_datarefs_alignment (NULL, bb_vinfo))
> - {
> - if (vect_print_dump_info (REPORT_UNVECTORIZED_LOCATIONS))
> - fprintf (vect_dump, "not vectorized: unsupported alignment in basic
> "
> - "block.\n");
> -
> - destroy_bb_vec_info (bb_vinfo);
> - return NULL;
> - }
> -
> /* Check the SLP opportunities in the basic block, analyze and build SLP
> trees. */
> if (!vect_analyze_slp (NULL, bb_vinfo))
> --- 2050,2055 ----
> *************** vect_slp_analyze_bb_1 (basic_block bb)
> *** 2082,2087 ****
> --- 2072,2087 ----
> vect_mark_slp_stmts_relevant (SLP_INSTANCE_TREE (instance));
> }
>
> + if (!vect_verify_datarefs_alignment (NULL, bb_vinfo))
> + {
> + if (vect_print_dump_info (REPORT_UNVECTORIZED_LOCATIONS))
> + fprintf (vect_dump, "not vectorized: unsupported alignment in basic
> "
> + "block.\n");
> +
> + destroy_bb_vec_info (bb_vinfo);
> + return NULL;
> + }
> +
> if (!vect_slp_analyze_operations (bb_vinfo))
> {
> if (vect_print_dump_info (REPORT_UNVECTORIZED_LOCATIONS))
>
> --
> Dr. Ulrich Weigand
> GNU Toolchain for Linux on System z and Cell BE
> ulrich.weig...@de.ibm.com
>
