* [PATCH] tree-optimization/105250 - adjust fold_convertible_p PR105140 fix @ 2022-04-13 7:46 Richard Biener 2022-04-13 8:04 ` Richard Sandiford 0 siblings, 1 reply; 6+ messages in thread From: Richard Biener @ 2022-04-13 7:46 UTC (permalink / raw) To: gcc-patches 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 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){}); +} -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tree-optimization/105250 - adjust fold_convertible_p PR105140 fix 2022-04-13 7:46 [PATCH] tree-optimization/105250 - adjust fold_convertible_p PR105140 fix Richard Biener @ 2022-04-13 8:04 ` Richard Sandiford 2022-04-13 8:48 ` Richard Biener 0 siblings, 1 reply; 6+ messages in thread From: Richard Sandiford @ 2022-04-13 8:04 UTC (permalink / raw) To: Richard Biener via Gcc-patches; +Cc: Richard Biener 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){}); > +} ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tree-optimization/105250 - adjust fold_convertible_p PR105140 fix 2022-04-13 8:04 ` Richard Sandiford @ 2022-04-13 8:48 ` Richard Biener 2022-04-13 8:55 ` Richard Biener 2022-04-13 10:59 ` Richard Sandiford 0 siblings, 2 replies; 6+ messages in thread From: Richard Biener @ 2022-04-13 8:48 UTC (permalink / raw) To: Richard Sandiford; +Cc: Richard Biener via Gcc-patches 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 <rguenther@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tree-optimization/105250 - adjust fold_convertible_p PR105140 fix 2022-04-13 8:48 ` Richard Biener @ 2022-04-13 8:55 ` Richard Biener 2022-04-13 9:06 ` Richard Biener 2022-04-13 10:59 ` Richard Sandiford 1 sibling, 1 reply; 6+ messages in thread From: Richard Biener @ 2022-04-13 8:55 UTC (permalink / raw) To: Richard Sandiford; +Cc: Richard Biener via Gcc-patches 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 <rguenther@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tree-optimization/105250 - adjust fold_convertible_p PR105140 fix 2022-04-13 8:55 ` Richard Biener @ 2022-04-13 9:06 ` Richard Biener 0 siblings, 0 replies; 6+ messages in thread From: Richard Biener @ 2022-04-13 9:06 UTC (permalink / raw) To: Richard Sandiford; +Cc: Richard Biener via Gcc-patches 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 Biener <rguenther@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tree-optimization/105250 - adjust fold_convertible_p PR105140 fix 2022-04-13 8:48 ` Richard Biener 2022-04-13 8:55 ` Richard Biener @ 2022-04-13 10:59 ` Richard Sandiford 1 sibling, 0 replies; 6+ messages in thread From: Richard Sandiford @ 2022-04-13 10:59 UTC (permalink / raw) To: Richard Biener; +Cc: Richard Biener via Gcc-patches 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-13 10:59 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-13 7:46 [PATCH] tree-optimization/105250 - adjust fold_convertible_p PR105140 fix Richard Biener 2022-04-13 8:04 ` Richard Sandiford 2022-04-13 8:48 ` Richard Biener 2022-04-13 8:55 ` Richard Biener 2022-04-13 9:06 ` Richard Biener 2022-04-13 10:59 ` Richard Sandiford
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).