| Message ID | nycvar.YFH.7.76.1909170941380.5566@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
| Series | Fix PR91790 | expand |
Hi! On Tue, Sep 17, 2019 at 09:45:54AM +0200, Richard Biener wrote: > The following fixes an old vectorizer issue with realignment support > (thus only powerpc is affected) and BB vectorization. The realignment > token is set up from the wrong data-ref which causes an SSA verification > failure but in other circumstances might simply generate wrong code. > > Bootstrap running on x86_64-unknown-linux-gnu, I'll install this > as obvious on trunk. > > PPC folks - you know best how to appropriately test a target > where we use the re-alignment optimization. IIRC on later > powerpc hardware this isn't exercised at all since we can use > unaligned accesses. > > The issue is at least present on the GCC 9 branch as well but I'd > appreciate testing where it exercises the path before considering > a backport. Is there a testcase? You can use -malign-natural to get stricter alignment requirements, that might help. Cc:ing Bill, this is vectorizer :-) Segher
On 9/19/19 1:34 PM, Segher Boessenkool wrote: > Hi! > > On Tue, Sep 17, 2019 at 09:45:54AM +0200, Richard Biener wrote: >> The following fixes an old vectorizer issue with realignment support >> (thus only powerpc is affected) and BB vectorization. The realignment >> token is set up from the wrong data-ref which causes an SSA verification >> failure but in other circumstances might simply generate wrong code. >> >> Bootstrap running on x86_64-unknown-linux-gnu, I'll install this >> as obvious on trunk. >> >> PPC folks - you know best how to appropriately test a target >> where we use the re-alignment optimization. IIRC on later >> powerpc hardware this isn't exercised at all since we can use >> unaligned accesses. >> >> The issue is at least present on the GCC 9 branch as well but I'd >> appreciate testing where it exercises the path before considering >> a backport. > Is there a testcase? Richard, can you turn the PR's reported test into a torture test case? We post P7 big-endian results frequently to gcc-testresults, and this bug hasn't fired on anything there, so it's not covered by existing tests. Nothing has turned up on the testers since your patch went in, so having the new test added should be sufficient, I'd think. P7 or older running big-endian is what's needed to test realignment support. Thanks, Bill > > You can use -malign-natural to get stricter alignment requirements, > that might help. > > Cc:ing Bill, this is vectorizer :-) > > > Segher
On Thu, 19 Sep 2019, Bill Schmidt wrote: > > On 9/19/19 1:34 PM, Segher Boessenkool wrote: > > Hi! > > > > On Tue, Sep 17, 2019 at 09:45:54AM +0200, Richard Biener wrote: > >> The following fixes an old vectorizer issue with realignment support > >> (thus only powerpc is affected) and BB vectorization. The realignment > >> token is set up from the wrong data-ref which causes an SSA verification > >> failure but in other circumstances might simply generate wrong code. > >> > >> Bootstrap running on x86_64-unknown-linux-gnu, I'll install this > >> as obvious on trunk. > >> > >> PPC folks - you know best how to appropriately test a target > >> where we use the re-alignment optimization. IIRC on later > >> powerpc hardware this isn't exercised at all since we can use > >> unaligned accesses. > >> > >> The issue is at least present on the GCC 9 branch as well but I'd > >> appreciate testing where it exercises the path before considering > >> a backport. > > Is there a testcase? > > > Richard, can you turn the PR's reported test into a torture test case? We > post P7 big-endian results frequently to gcc-testresults, and this bug hasn't > fired on anything there, so it's not covered by existing tests. Nothing has > turned up on the testers since your patch went in, so having the new test > added should be sufficient, I'd think. P7 or older running big-endian is > what's needed to test realignment support. I was hoping you guys could take a stab at that, eventually also creating a wrong-code runnable testcase (one that would alignment-fault at runtime for using the wrong alignment token). The testcase in the PR is unwieldly, aka C++, and it likely requires machine specific options to more reliably trigger - IIRC I needed -mcpu=e300c3 or so thus apply subtarget specific tuning. OTOH the patch was so obvious... Richard. > Thanks, > > Bill > > > > > You can use -malign-natural to get stricter alignment requirements, > > that might help. > > > > Cc:ing Bill, this is vectorizer :-) > > > > > > Segher >
Index: gcc/tree-vect-stmts.c =================================================================== --- gcc/tree-vect-stmts.c (revision 275747) +++ gcc/tree-vect-stmts.c (working copy) @@ -9169,7 +9169,9 @@ vectorizable_load (stmt_vec_info stmt_in || alignment_support_scheme == dr_explicit_realign) && !compute_in_loop) { - msq = vect_setup_realignment (first_stmt_info, gsi, &realignment_token, + msq = vect_setup_realignment (first_stmt_info_for_drptr + ? first_stmt_info_for_drptr + : first_stmt_info, gsi, &realignment_token, alignment_support_scheme, NULL_TREE, &at_loop); if (alignment_support_scheme == dr_explicit_realign_optimized)