Bug 81362 - [8 regression] FAIL: gcc.dg/vect/no-vfa-vect-57.c execution test
Summary: [8 regression] FAIL: gcc.dg/vect/no-vfa-vect-57.c execution test
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: 8.0
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2017-07-08 19:43 UTC by Andreas Schwab
Modified: 2017-12-16 17:00 UTC (History)
2 users (show)

See Also:
Host:
Target: powerpc64-*-*
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
no-vfa-vect-57.c.159t.vect (5.04 KB, text/plain)
2017-07-10 17:01 UTC, Andreas Schwab
Details
no-vfa-vect-57.s (1.35 KB, text/plain)
2017-07-10 17:02 UTC, Andreas Schwab
Details
Tentative patch (1010 bytes, patch)
2017-07-11 15:57 UTC, rdapp
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Schwab 2017-07-08 19:43:46 UTC
This fails on G5.

http://gcc.gnu.org/ml/gcc-testresults/2017-06/msg00297.html

Broken by r248680 (Vector peeling cost model 6/6).

(gdb) bt
#0  0x00003fffb7d3cc00 in __GI_raise (sig=<optimized out>)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00003fffb7d3e8bc in __GI_abort () at abort.c:89
#2  0x0000000010000870 in bar (pa=pa@entry=0x3fffffffe680, 
    pb=pb@entry=0x3fffffffe200, pc=pc@entry=0x3fffffffde00)
    at /daten/gcc/gcc-20170602/gcc/testsuite/gcc.dg/vect/no-vfa-vect-57.c:18
#3  0x0000000010000b20 in main1 (pa=pa@entry=0x3fffffffe680)
    at /daten/gcc/gcc-20170602/gcc/testsuite/gcc.dg/vect/no-vfa-vect-57.c:58
#4  0x0000000010000618 in main ()
    at /daten/gcc/gcc-20170602/gcc/testsuite/gcc.dg/vect/no-vfa-vect-57.c:69
Comment 1 rdapp 2017-07-10 07:13:04 UTC
Could you provide the vectorizer dump (-fdump-tree-vect-details)? The generated assembly might also be interesting as well as the exact command line for building (in the test suite logs).  I compiled --with-cpu-64=power4 yet cannot reproduce the FAIL.  For me, the compile options set by the test suite are

-maltivec -mvsx -mno-allow-movmisalign -ftree-vectorize -fno-vect-cost-model -fno-common -O2 -fdump-tree-vect-details --param vect-max-version-for-alias-checks=0 -lm -m32
Comment 2 Andreas Schwab 2017-07-10 17:01:25 UTC
Created attachment 41708 [details]
no-vfa-vect-57.c.159t.vect
Comment 3 Andreas Schwab 2017-07-10 17:02:14 UTC
Created attachment 41709 [details]
no-vfa-vect-57.s
Comment 4 Andreas Schwab 2017-07-10 17:04:31 UTC
There is no VSX on G5.
Comment 5 rdapp 2017-07-11 07:54:49 UTC
Ah, npeel is set by vect_peeling_hash_get_lowest_cost although the corresponding dr is not used afterwards.  It should be save to get rid of the npeel parameter since we use the returned peeling's npeel anyway.  I think the same is true for 
body_cost_vec but it's not used afterwards so doesn't cause problems.

The following fixes the regression for me:

index 5103ba1..257be41 100644                                                                                                                                                      [0/92965]
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -1347,7 +1347,6 @@ vect_peeling_hash_get_lowest_cost (_vect_peel_info **slot,
 static struct _vect_peel_extended_info
 vect_peeling_hash_choose_best_peeling (hash_table<peel_info_hasher> *peeling_htab,
                       loop_vec_info loop_vinfo,
-                                       unsigned int *npeel,
                       stmt_vector_for_cost *body_cost_vec)
 {
    struct _vect_peel_extended_info res;
@@ -1371,7 +1370,6 @@ vect_peeling_hash_choose_best_peeling (hash_table<peel_info_hasher> *pee
ling_hta
        res.outside_cost = 0;
      }
 
-   *npeel = res.peel_info.npeel;
    *body_cost_vec = res.body_cost_vec;
    return res;
 }
@@ -1812,7 +1810,7 @@ vect_enhance_data_refs_alignment (loop_vec_info loop_vinfo)
          unless aligned.  So we try to choose the best possible peeling from
     the hash table.  */
       peel_for_known_alignment = vect_peeling_hash_choose_best_peeling
-   (&peeling_htab, loop_vinfo, &npeel, &body_cost_vec);
+   (&peeling_htab, loop_vinfo, &body_cost_vec);
     }
 
   /* Compare costs of peeling for known and unknown alignment. */
Comment 6 rdapp 2017-07-11 15:57:19 UTC
Created attachment 41715 [details]
Tentative patch

Removed the npeel function argument, also removed body_cost_vec and the corresponding release ()es.  I'm not so sure about the semantics of vecs in general.  Are we leaking memory when not releasing the body_cost_vecs contained in the various _vect_peel_extended_infos?  If it was RAII-like body_cost_vec there would have been no need to release () body_cost_vec before, so I assume it is not.
Comment 7 Andreas Schwab 2017-07-11 18:11:21 UTC
This fixes both no-vfa-vect-57.c and no-vfa-vect-61.c for -m64 and -m32.
Comment 8 Segher Boessenkool 2017-11-19 22:41:14 UTC
Is this fixed now?
Comment 9 Jeffrey A. Law 2017-12-16 17:00:41 UTC
This was fixed back in July:

commit f0f5171608d68c3cb3aa6aa43d64814d4f9d67d5
Author: krebbel <krebbel@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Tue Jul 18 09:23:35 2017 +0000

    Fix PR81362: Vector peeling
    
    npeel was erroneously overwritten by vect_peeling_hash_get_lowest_cost
    although the corresponding dataref is not used afterwards.  It should
    be safe to get rid of the npeel parameter since we use the returned
    peeling_info's npeel anyway.  Also removed the body_cost_vec parameter
    which is not used elsewhere.
    
    gcc/ChangeLog:
    
    2017-07-18  Robin Dapp  <rdapp@linux.vnet.ibm.com>
    
            * tree-vect-data-refs.c (vect_enhance_data_refs_alignment): Remove
            body_cost_vec from _vect_peel_extended_info.
            (vect_peeling_hash_get_lowest_cost): Do not set body_cost_vec.
            (vect_peeling_hash_choose_best_peeling): Remove body_cost_vec and
            npeel.