| Message ID | patch-18851-tamar@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | middle-end: Save VMAT info in stmt_vec_info as well for SLP for costing. | expand |
On Mon, 14 Oct 2024, Tamar Christina wrote: > Hi All, > > While chasing down a costing discrepancy between SLP and non-SLP noticed that > costing for different VMATs were not working. > > It looks like the vectorizer for non-SLP stores the VMAT type in > STMT_VINFO_MEMORY_ACCESS_TYPE on the stmt_info, but for SLP it stores it in > SLP_TREE_MEMORY_ACCESS_TYPE which is on the SLP node itself. > > While I could fairly easily fix the AArch64 backend to look at both, I had a > look at how various targets use the the macros. At the moment there are 4 > backends with costing depending on specific VMAT: i386, rs6000, rvv and > AArch64. Of these 4 only i386 actually also reads SLP_TREE_MEMORY_ACCESS_TYPE. > > Some of the targets look like it's gonna be a bit of churn to read and the > fact that there is two location to check makes it somewhat annoying. > > Instead I'm proposing that, for at least during the transition period that we > also store the VMAT in the SLP tree's representive statement. This restores > the costing in the specific backends. > > If not OK, I'm happy to just fix AArch64. > > Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu -m32, > -m64 and no issues. > > Ok for master? No, this will cause issues - it was removed there specifically because it will break when a DR is both SLP and not SLP vectorized. git blame will tell you. So the "fix" is to indeed look at SLP_TREE_MEMORY_ACCESS_TYPE if the cost hook gets passed a slp_node and STMT_VINFO_MEMORY_ACCESS_TYPE if not. Maybe instead add an inline helper to tree-vectorizer.h doing this switching like inline vect_memory_access_type vect_memory_access_type (stmt_vec_info stmt_info, slp_tree node) { if (node) return SLP_TREE_MEMORY_ACCESS_TYPE (node); else return STMT_VINFO_MEMORY_ACCESS_TYPE (stmt_info); } > > Thanks, > Tamar > > gcc/ChangeLog: > > * tree-vect-stmts.cc (vectorizable_load): Always set > STMT_VINFO_MEMORY_ACCESS_TYPE. > > --- > diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc > index 4f6905f15417f90c6f36e1711a7a25071f0f507c..22f32598059377ed5285fab018b342c8c286a441 100644 > --- a/gcc/tree-vect-stmts.cc > +++ b/gcc/tree-vect-stmts.cc > @@ -10235,9 +10235,8 @@ vectorizable_load (vec_info *vinfo, > return false; > } > > - if (!slp) > - STMT_VINFO_MEMORY_ACCESS_TYPE (stmt_info) = memory_access_type; > - else > + STMT_VINFO_MEMORY_ACCESS_TYPE (stmt_info) = memory_access_type; > + if (slp) > SLP_TREE_MEMORY_ACCESS_TYPE (slp_node) = memory_access_type; > > if (loop_vinfo > > > > >
diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc index 4f6905f15417f90c6f36e1711a7a25071f0f507c..22f32598059377ed5285fab018b342c8c286a441 100644 --- a/gcc/tree-vect-stmts.cc +++ b/gcc/tree-vect-stmts.cc @@ -10235,9 +10235,8 @@ vectorizable_load (vec_info *vinfo, return false; } - if (!slp) - STMT_VINFO_MEMORY_ACCESS_TYPE (stmt_info) = memory_access_type; - else + STMT_VINFO_MEMORY_ACCESS_TYPE (stmt_info) = memory_access_type; + if (slp) SLP_TREE_MEMORY_ACCESS_TYPE (slp_node) = memory_access_type; if (loop_vinfo