diff mbox

[RFC] S/390: Alignment peeling prolog generation

Message ID CAFiYyc18gq4g+v-RFQcQ=-1TRh=RraE8_wPXNvdO58NSGnjoHQ@mail.gmail.com
State New
Headers show

Commit Message

Richard Biener May 11, 2017, 12:14 p.m. UTC 
On Thu, May 11, 2017 at 1:15 PM, Robin Dapp <rdapp@linux.vnet.ibm.com> wrote:
> Included the requested changes in the patches (to follow).  I removed
> the alignment count check now altogether.
>
>> I'm not sure why you test for unlimited_cost_model here as I said
>> elsewhere I'm not sure
>> what not cost modeling means for static decisions.  The purpose of
>> unlimited_cost_model
>> is to always vectorize when possible and omit the runtime
>> profitability check.  So for peeling
>> I'd just always use the cost model.  Thus please drop this check.
>
> Without that, I get one additional FAIL gcc.dg/vect/slp-25.c for x86.
> It is caused by choosing no peeling (inside costs 0) over peeling for
> known alignment with unlimited cost model (inside costs 0 as well).
> Costs 0 for no peeling are caused by count == 0 or rather ncopies = vf /
> nunits == 4 / 8 == 0 in record_stmt_costs ().  Shouldn't always hold
> ncopies > 0? Even 0.5 would have worked here to make no peeling more
> expensive than 0.

That's odd.  I can't get


to ICE with the testcase (unpatched trunk)

Where's that record_stmt_cost call done?  You can't simply use vf/nunits
for SLP.

Richard.

> Test suite on s390x is clean.
>
> Regards
>  Robin
>

Comments

Richard Biener May 11, 2017, 12:15 p.m. UTC | #1 
On Thu, May 11, 2017 at 2:14 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Thu, May 11, 2017 at 1:15 PM, Robin Dapp <rdapp@linux.vnet.ibm.com> wrote:
>> Included the requested changes in the patches (to follow).  I removed
>> the alignment count check now altogether.
>>
>>> I'm not sure why you test for unlimited_cost_model here as I said
>>> elsewhere I'm not sure
>>> what not cost modeling means for static decisions.  The purpose of
>>> unlimited_cost_model
>>> is to always vectorize when possible and omit the runtime
>>> profitability check.  So for peeling
>>> I'd just always use the cost model.  Thus please drop this check.
>>
>> Without that, I get one additional FAIL gcc.dg/vect/slp-25.c for x86.
>> It is caused by choosing no peeling (inside costs 0) over peeling for
>> known alignment with unlimited cost model (inside costs 0 as well).
>> Costs 0 for no peeling are caused by count == 0 or rather ncopies = vf /
>> nunits == 4 / 8 == 0 in record_stmt_costs ().  Shouldn't always hold
>> ncopies > 0? Even 0.5 would have worked here to make no peeling more
>> expensive than 0.
>
> That's odd.  I can't get
>
> Index: gcc/tree-vect-stmts.c
> ===================================================================
> --- gcc/tree-vect-stmts.c       (revision 247882)
> +++ gcc/tree-vect-stmts.c       (working copy)
> @@ -95,6 +96,7 @@ record_stmt_cost (stmt_vector_for_cost *
>                   enum vect_cost_for_stmt kind, stmt_vec_info stmt_info,
>                   int misalign, enum vect_cost_model_location where)
>  {
> +  gcc_assert (count > 0);
>    if (body_cost_vec)
>      {
>        tree vectype = stmt_info ? stmt_vectype (stmt_info) : NULL_TREE;
>
> to ICE with the testcase (unpatched trunk)
>
> Where's that record_stmt_cost call done?  You can't simply use vf/nunits
> for SLP.

Ah, of course needs -fvect-cost-model.

I'll investigate.

Richard.

> Richard.
>
>> Test suite on s390x is clean.
>>
>> Regards
>>  Robin
>>
diff mbox

Patch

Index: gcc/tree-vect-stmts.c
===================================================================
--- gcc/tree-vect-stmts.c       (revision 247882)
+++ gcc/tree-vect-stmts.c       (working copy)
@@ -95,6 +96,7 @@  record_stmt_cost (stmt_vector_for_cost *
                  enum vect_cost_for_stmt kind, stmt_vec_info stmt_info,
                  int misalign, enum vect_cost_model_location where)
 {
+  gcc_assert (count > 0);
   if (body_cost_vec)
     {
       tree vectype = stmt_info ? stmt_vectype (stmt_info) : NULL_TREE;