| Message ID | mptttlyfa7y.fsf@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | vect: Tighten check for impossible SLP layouts [PR113205] | expand |
> Am 24.02.2024 um 11:06 schrieb Richard Sandiford <richard.sandiford@arm.com>: > > During its forward pass, the SLP layout code tries to calculate > the cost of a layout change on an incoming edge. This is taken > as the minimum of two costs: one in which the source partition > keeps its current layout (chosen earlier during the pass) and > one in which the source partition switches to the new layout. > The latter can sometimes be arranged by the backward pass. > > If only one of the costs is valid, the other cost was ignored. > But the PR shows that this is not safe. If the source partition > has layout 0 (the normal layout), we have to be prepared to handle > the case in which that ends up being the only valid layout. > > Other code already accounts for this restriction, e.g. see > the code starting with: > > /* Reject the layout if it would make layout 0 impossible > for later partitions. This amounts to testing that the > target supports reversing the layout change on edges > to later partitions. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? Ok Thanks, Richard > Richard > > > gcc/ > PR tree-optimization/113205 > * tree-vect-slp.cc (vect_optimize_slp_pass::forward_cost): Reject > the proposed layout if it does not allow a source partition with > layout 2 to keep that layout. > > gcc/testsuite/ > PR tree-optimization/113205 > * gcc.dg/torture/pr113205.c: New test. > --- > gcc/testsuite/gcc.dg/torture/pr113205.c | 19 +++++++++++++++++++ > gcc/tree-vect-slp.cc | 4 ++++ > 2 files changed, 23 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/torture/pr113205.c > > diff --git a/gcc/testsuite/gcc.dg/torture/pr113205.c b/gcc/testsuite/gcc.dg/torture/pr113205.c > new file mode 100644 > index 00000000000..edfba7fcd0e > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/torture/pr113205.c > @@ -0,0 +1,19 @@ > +char a; > +char *b, *c; > +int d, e, f, g, h; > +int *i; > + > +void > +foo (void) > +{ > + unsigned p; > + d = i[0]; > + e = i[1]; > + f = i[2]; > + g = i[3]; > + p = d * b[0]; > + p += f * c[h]; > + p += e * b[h]; > + p += g * c[h]; > + a = (p + 8000) >> (__SIZEOF_INT__ * __CHAR_BIT__ / 2); > +} > diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc > index 7cf9504398c..895f4f7fb6b 100644 > --- a/gcc/tree-vect-slp.cc > +++ b/gcc/tree-vect-slp.cc > @@ -5034,6 +5034,10 @@ vect_optimize_slp_pass::forward_cost (graph_edge *ud, unsigned int from_node_i, > cost.split (from_partition.out_degree); > cost.add_serial_cost (edge_cost); > } > + else if (from_partition.layout == 0) > + /* We must allow the source partition to have layout 0 as a fallback, > + in case all other options turn out to be impossible. */ > + return cost; > > /* Take the minimum of that cost and the cost that applies if > FROM_PARTITION instead switches to TO_LAYOUT_I. */ > -- > 2.25.1 >
diff --git a/gcc/testsuite/gcc.dg/torture/pr113205.c b/gcc/testsuite/gcc.dg/torture/pr113205.c new file mode 100644 index 00000000000..edfba7fcd0e --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr113205.c @@ -0,0 +1,19 @@ +char a; +char *b, *c; +int d, e, f, g, h; +int *i; + +void +foo (void) +{ + unsigned p; + d = i[0]; + e = i[1]; + f = i[2]; + g = i[3]; + p = d * b[0]; + p += f * c[h]; + p += e * b[h]; + p += g * c[h]; + a = (p + 8000) >> (__SIZEOF_INT__ * __CHAR_BIT__ / 2); +} diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc index 7cf9504398c..895f4f7fb6b 100644 --- a/gcc/tree-vect-slp.cc +++ b/gcc/tree-vect-slp.cc @@ -5034,6 +5034,10 @@ vect_optimize_slp_pass::forward_cost (graph_edge *ud, unsigned int from_node_i, cost.split (from_partition.out_degree); cost.add_serial_cost (edge_cost); } + else if (from_partition.layout == 0) + /* We must allow the source partition to have layout 0 as a fallback, + in case all other options turn out to be impossible. */ + return cost; /* Take the minimum of that cost and the cost that applies if FROM_PARTITION instead switches to TO_LAYOUT_I. */
During its forward pass, the SLP layout code tries to calculate the cost of a layout change on an incoming edge. This is taken as the minimum of two costs: one in which the source partition keeps its current layout (chosen earlier during the pass) and one in which the source partition switches to the new layout. The latter can sometimes be arranged by the backward pass. If only one of the costs is valid, the other cost was ignored. But the PR shows that this is not safe. If the source partition has layout 0 (the normal layout), we have to be prepared to handle the case in which that ends up being the only valid layout. Other code already accounts for this restriction, e.g. see the code starting with: /* Reject the layout if it would make layout 0 impossible for later partitions. This amounts to testing that the target supports reversing the layout change on edges to later partitions. Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? Richard gcc/ PR tree-optimization/113205 * tree-vect-slp.cc (vect_optimize_slp_pass::forward_cost): Reject the proposed layout if it does not allow a source partition with layout 2 to keep that layout. gcc/testsuite/ PR tree-optimization/113205 * gcc.dg/torture/pr113205.c: New test. --- gcc/testsuite/gcc.dg/torture/pr113205.c | 19 +++++++++++++++++++ gcc/tree-vect-slp.cc | 4 ++++ 2 files changed, 23 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/torture/pr113205.c