tree-optimization/105250 - adjust fold_convertible_p PR105140 fix

Message ID 20220413074636.9269E13A91@imap2.suse-dmz.suse.de
State Committed
Commit 4e892de6774f86540d36385701aa7b0a2bba5155
Headers
Series tree-optimization/105250 - adjust fold_convertible_p PR105140 fix |

Commit Message

Richard Biener April 13, 2022, 7:46 a.m. UTC
  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  <rguenther@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.

	* 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
  

Comments

Richard Sandiford April 13, 2022, 8:04 a.m. UTC | #1
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  <rguenther@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.

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 April 13, 2022, 8:48 a.m. UTC | #2
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  <rguenther@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 April 13, 2022, 8:55 a.m. UTC | #3
On Wed, 13 Apr 2022, Richard Biener wrote:

> 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  <rguenther@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?

Btw, fold_convertible_p is currently used in few places only:

fold-const.cc:fold_convertible_p (const_tree type, const_tree arg)
ipa-cp.cc:      || fold_convertible_p (param_type, value))
ipa-param-manipulation.cc:            if (!fold_convertible_p (TREE_TYPE 
(origin), arg))
ipa-prop.cc:      if (fold_convertible_p (TREE_TYPE (rhs), v->value))
tree-inline.cc:  if (fold_convertible_p (type, value))
tree-inline.cc:      if (fold_convertible_p (caller_type, var))

all in places that try to deal with type mismatches in IPA (from
parameters).

Richard.

> 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 April 13, 2022, 9:06 a.m. UTC | #4
On Wed, 13 Apr 2022, Richard Biener wrote:

> On Wed, 13 Apr 2022, Richard Biener wrote:
> 
> > 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  <rguenther@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?
> 
> Btw, fold_convertible_p is currently used in few places only:
> 
> fold-const.cc:fold_convertible_p (const_tree type, const_tree arg)
> ipa-cp.cc:      || fold_convertible_p (param_type, value))
> ipa-param-manipulation.cc:            if (!fold_convertible_p (TREE_TYPE 
> (origin), arg))
> ipa-prop.cc:      if (fold_convertible_p (TREE_TYPE (rhs), v->value))
> tree-inline.cc:  if (fold_convertible_p (type, value))
> tree-inline.cc:      if (fold_convertible_p (caller_type, var))
> 
> all in places that try to deal with type mismatches in IPA (from
> parameters).

Btw, I can't find a tree_int_cst_equal replacement that would work
for POLY_INT_CST as well as INTEGER_CST, is there any that I
missed?

Richard.

> Richard.
> 
> > 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 Sandiford April 13, 2022, 10:59 a.m. UTC | #5
Richard Biener <rguenther@suse.de> writes:
> 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  <rguenther@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.

Ah, right, sorry for the false alarm.  I'd read the comment as saying
that it determined when nop/convert_exprs were valid, rather than
whether fold_convert could handle them.

> 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?

Yeah, I guess it makes sense to keep things as they are,
with code that is aware of these vector conversions using
supportable_convert_operation.

It tests fine on SVE, as you expected.

Thanks,
Richard
  

Patch

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){});
+}