Bug 102467 - Missed SLP discovery for gathers
Summary: Missed SLP discovery for gathers
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 12.0
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks: vectorizer
  Show dependency treegraph
 
Reported: 2021-09-23 11:17 UTC by Richard Biener
Modified: 2021-12-02 17:21 UTC (History)
1 user (show)

See Also:
Host:
Target: aarch64
Build:
Known to work:
Known to fail:
Last reconfirmed: 2021-09-29 00:00:00


Attachments
half-way patch (1.86 KB, patch)
2021-09-23 11:34 UTC, Richard Biener
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2021-09-23 11:17:57 UTC
void foo (double * __restrict a, long * __restrict b, double * c)
{
  for (int i = 0; i < 1024; ++i)
    {
      a[2*i] = c[b[2*i]];
      a[2*i + 1] = c[b[2*i + 1]];
    }
}

is not SLP vectorized.
Comment 1 Richard Biener 2021-09-23 11:18:34 UTC
"easiest" to do with the IFN scheme since then SLP discovery already sees IFN_GATHERs as discovered by pattern recog.
Comment 2 Richard Biener 2021-09-23 11:34:30 UTC
Created attachment 51504 [details]
half-way patch

A start, lacks adjustment of the code generation part which doesn't expect SLP at the moment.
Comment 3 Andrew Pinski 2021-09-29 00:11:33 UTC
Confirmed.
Comment 4 GCC Commits 2021-11-30 09:54:03 UTC
The master branch has been updated by Richard Sandiford <rsandifo@gcc.gnu.org>:

https://gcc.gnu.org/g:10833849b55401a52f2334eb032a70beb688e9fc

commit r12-5612-g10833849b55401a52f2334eb032a70beb688e9fc
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Tue Nov 30 09:52:29 2021 +0000

    vect: Support gather loads with SLP
    
    This patch adds SLP support for IFN_GATHER_LOAD.  Like the SLP
    support for IFN_MASK_LOAD, it works by treating only some of the
    arguments as child nodes.  Unlike IFN_MASK_LOAD, it requires the
    other arguments (base, scale, and extension type) to be the same
    for all calls in the group.  It does not require/expect the loads
    to be in a group (which probably wouldn't make sense for gathers).
    
    I was worried about the possible alias effect of moving gathers
    around to be part of the same SLP group.  The patch therefore
    makes vect_analyze_data_ref_dependence treat gathers and scatters
    as a top-level concern, punting if the accesses aren't completely
    independent and if the user hasn't told us that a particular
    VF is safe.  I think in practice we already punted in the same
    circumstances; the idea is just to make it more explicit.
    
    gcc/
            PR tree-optimization/102467
            * doc/sourcebuild.texi (vect_gather_load_ifn): Document.
            * tree-vect-data-refs.c (vect_analyze_data_ref_dependence):
            Commonize safelen handling.  Punt for anything involving
            gathers and scatters unless safelen says otherwise.
            * tree-vect-slp.c (arg1_map): New variable.
            (vect_get_operand_map): Handle IFN_GATHER_LOAD.
            (vect_build_slp_tree_1): Likewise.
            (vect_build_slp_tree_2): Likewise.
            (compatible_calls_p): If vect_get_operand_map returns nonnull,
            check that any skipped arguments are equal.
            (vect_slp_analyze_node_operations_1): Tighten reduction check.
            * tree-vect-stmts.c (check_load_store_for_partial_vectors): Take
            an ncopies argument.
            (vect_get_gather_scatter_ops): Take slp_node and ncopies arguments.
            Handle SLP nodes.
            (vectorizable_store, vectorizable_load): Adjust accordingly.
    
    gcc/testsuite/
            * lib/target-supports.exp
            (check_effective_target_vect_gather_load_ifn): New target test.
            * gcc.dg/vect/vect-gather-1.c: New test.
            * gcc.dg/vect/vect-gather-2.c: Likewise.
            * gcc.target/aarch64/sve/gather_load_11.c: Likewise.
Comment 5 Richard Sandiford 2021-11-30 12:00:36 UTC
Fixed for IFNs.  Not sure whether we want to keep this open for
the built-in route too.
Comment 6 rguenther@suse.de 2021-11-30 12:06:14 UTC
On Tue, 30 Nov 2021, rsandifo at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102467
> 
> rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |rsandifo at gcc dot gnu.org
> 
> --- Comment #5 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
> Fixed for IFNs.  Not sure whether we want to keep this open for
> the built-in route too.

I don't think so.  The fix for the built-in route is to get rid of it.
Of course I miserably failed at that - meh :/
Comment 7 Richard Sandiford 2021-12-02 17:21:08 UTC
OK, closing as fixed then :-)