On Wed, 13 Apr 2022, Richard Sandiford wrote:
> Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > The following reverts the original PR105140 fix and goes for instead
> > applying the additional fold_convert constraint for VECTOR_TYPE
> > conversions also to fold_convertible_p. I did not try sanitizing
> > all of this at this point.
> >
> > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> >
> > 2022-04-13 Richard Biener <rguent...@suse.de>
> >
> > PR tree-optimization/105250
> > * fold-const.cc (fold_convertible_p): Revert
> > r12-7979-geaaf77dd85c333, instead check for size equality
> > of the vector types involved.
>
> This doesn't look right, and I think it'll break SVE. For one
> thing, the tree_int_cst_equal check is bound to fail for
> variable-length vectors.
>
> But also, the idea was to allow element-wise conversions between
> different vector sizes. For example, you can do a nop/convert
> from V4SI to V4DI, which converts 4 SIs to 4 DIs. This is used
> a lot for conversions to and from “partial” SVE vectors, where smaller
> elements are stored in wider containers.
But fold_convertible_p is used as guard for fold_convert in a lot of
places and that will simply ICE when there's a mismatch in size
as can be seen in the testcase. Note the code as before the
previous fix couldn't really have worked as expected. Is there any
testcase that will "break" now?
I realize the fold_convertible_p comment says "using a NOP_EXPR" which
means it might conver a narrower set of conversions than fold_convert
(which will happily use FLOAT_EXPR and friends), but still it should
allow fold_convert to build the conversion.
The alternative would have been to emit a NOP_EXPR from fold_convert
for vector type conversions (with the correct constraints), but then
not all targets support those, so we'd need a target support check
in fold_convertible_p then?
Richard.
> Thanks,
> Richard
>
> >
> > * gcc.dg/pr105250.c: New testcase.
> > ---
> > gcc/fold-const.cc | 7 +++----
> > gcc/testsuite/gcc.dg/pr105250.c | 29 +++++++++++++++++++++++++++++
> > 2 files changed, 32 insertions(+), 4 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.dg/pr105250.c
> >
> > diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
> > index 7226bc5af01..a57ad0739fb 100644
> > --- a/gcc/fold-const.cc
> > +++ b/gcc/fold-const.cc
> > @@ -2379,13 +2379,12 @@ build_zero_vector (tree type)
> > return build_vector_from_val (type, t);
> > }
> >
> > -/* Returns true, if ARG, an operand or a type, is convertible to TYPE
> > - using a NOP_EXPR. */
> > +/* Returns true, if ARG is convertible to TYPE using a NOP_EXPR. */
> >
> > bool
> > fold_convertible_p (const_tree type, const_tree arg)
> > {
> > - const_tree orig = TYPE_P (arg) ? arg : TREE_TYPE (arg);
> > + const_tree orig = TREE_TYPE (arg);
> >
> > if (type == orig)
> > return true;
> > @@ -2417,7 +2416,7 @@ fold_convertible_p (const_tree type, const_tree arg)
> > return (VECTOR_TYPE_P (orig)
> > && known_eq (TYPE_VECTOR_SUBPARTS (type),
> > TYPE_VECTOR_SUBPARTS (orig))
> > - && fold_convertible_p (TREE_TYPE (type), TREE_TYPE (orig)));
> > + && tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (orig)));
> >
> > default:
> > return false;
> > diff --git a/gcc/testsuite/gcc.dg/pr105250.c
> > b/gcc/testsuite/gcc.dg/pr105250.c
> > new file mode 100644
> > index 00000000000..665dd95d8cb
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/pr105250.c
> > @@ -0,0 +1,29 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-w -Wno-psabi -O2" } */
> > +
> > +typedef int __attribute__((__vector_size__(4))) T;
> > +typedef int __attribute__((__vector_size__(8))) U;
> > +typedef int __attribute__((__vector_size__(16))) V;
> > +typedef int __attribute__((__vector_size__(32))) W;
> > +typedef _Float32 __attribute__((__vector_size__(16))) F;
> > +typedef _Float64 __attribute__((__vector_size__(32))) G;
> > +void foo();
> > +
> > +foo(int, int, int, int, U, U, V, V, W, W, int,
> > + T, int, U, U, V, V, W, W, T,
> > + T, int, U, U, V, V, W, W, T,
> > + T, int, W, W, T, T, int, int, int,
> > + int, int, int, W, int, int, int, int, int, int,
> > + V, W, T, int, int, U, F, int, int, int,
> > + int, int, int, G)
> > +{
> > + foo(0, 0, 0, 0, (U){}, (U){}, (V){}, (V){}, (W){},
> > + (W){}, 2, (T){}, 0, 0, 0, 0, (U){}, (U){},
> > + (V){}, (V){}, (W){}, (W){}, (T){},
> > + (T){}, 0, 0, 0, 0, (U){}, (U){}, (V){},
> > + (V){}, (W){}, (W){}, (T){}, (T){}, 0, 0, 0,
> > + 0, 0, 0, (T){},
> > + (T){}, (W){},
> > + (W){}, (T){}, (T){}, 0, 0, 0, 0, 0, 0, (W){},
> > + (V){}, (W){}, (T){}, 0, 0, (U){}, (F){});
> > +}
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
