RE: [PATCH PR94784] ICE: in simplify_vector_constructor, at tree-ssa-forwprop.c:2482

2020-04-27 Thread Yangfei (Felix)
> -Original Message-
> From: Richard Sandiford [mailto:richard.sandif...@arm.com]
> Sent: Monday, April 27, 2020 6:10 PM
> To: Yangfei (Felix) 
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH PR94784] ICE: in simplify_vector_constructor, at tree-
> ssa-forwprop.c:2482
> 
> > Good suggestion.  Modified accordingly.
> >
> >> LGTM otherwise, and sorry for the breakage.
> >
> > Does the v2 patch look better?
> >
> > Manually run the following three tests with runtest:
> > gcc.target/aarch64/sve/acle/general/pr94683.c
> > gcc.target/aarch64/sve/acle/general/pr94700.c
> > gcc.dg/pr94784.c
> 
> Thanks, pushed to master after testing on aarch64-linux-gnu.

My local bootstrap and regression tests for the v2 patch have just finished.  
The results looks good too.  Thanks for the efforts : - )

Best regards,
Felix
 





Re: [PATCH PR94784] ICE: in simplify_vector_constructor, at tree-ssa-forwprop.c:2482

2020-04-27 Thread Richard Sandiford
"Yangfei (Felix)"  writes:
>> We don't need to fall back to constant comparisons here.
>> We should assert for known_eq between the TYPE_VECTOR_SUBPARTS
>> instead.
>> 
>> Same for the other assert.
>
> Good suggestion.  Modified accordingly.
>
>> LGTM otherwise, and sorry for the breakage.
>
> Does the v2 patch look better?
>
> Manually run the following three tests with runtest:
> gcc.target/aarch64/sve/acle/general/pr94683.c
> gcc.target/aarch64/sve/acle/general/pr94700.c
> gcc.dg/pr94784.c

Thanks, pushed to master after testing on aarch64-linux-gnu.

Richard


RE: [PATCH PR94784] ICE: in simplify_vector_constructor, at tree-ssa-forwprop.c:2482

2020-04-27 Thread Yangfei (Felix)
> -Original Message-
> From: Richard Sandiford [mailto:richard.sandif...@arm.com]
> Sent: Monday, April 27, 2020 3:51 PM
> To: Yangfei (Felix) 
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH PR94784] ICE: in simplify_vector_constructor, at tree-
> ssa-forwprop.c:2482
> 
> "Yangfei (Felix)"  writes:
> > Hi,
> >
> > PR tree-optimization/94269
> > * tree-ssa-math-opts.c (convert_plusminus_to_widen): Restrict
> > -   this
> > -   operation to single basic block.
> > +   this operation to single basic block.
> 
> Not wrong, but might as well leave the entry as-is.

OK.  Leave it there.

> >
> We don't need to fall back to constant comparisons here.
> We should assert for known_eq between the TYPE_VECTOR_SUBPARTS
> instead.
> 
> Same for the other assert.

Good suggestion.  Modified accordingly.

> LGTM otherwise, and sorry for the breakage.

Does the v2 patch look better?

Manually run the following three tests with runtest:
gcc.target/aarch64/sve/acle/general/pr94683.c
gcc.target/aarch64/sve/acle/general/pr94700.c
gcc.dg/pr94784.c

Thanks for your help,
Felix


pr94784-v2.diff
Description: pr94784-v2.diff


Re: [PATCH PR94784] ICE: in simplify_vector_constructor, at tree-ssa-forwprop.c:2482

2020-04-27 Thread Richard Sandiford
"Yangfei (Felix)"  writes:
> Hi,
>
>   I see one gcc_assert was introduce in:  
> https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544271.html
>   This is causing an ICE for certain cases.  I have created a PR for this: 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94784
>   I did some check and it looks like everything works fine before the ICE.  
> In the testcase we have two vectors with the same ABI identity but with 
> different TYPE_MODEs.
>   The proposed patch flips the assert around so that it checks that the two 
> vectors have equal TYPE_VECTOR_SUBPARTS and that converting the corresponding 
> element types is a useless_type_conversion_p.
>
>   Bootstrap and tested on aarch64-linux-gnu.  OK?
>
> Thanks,
> Felix
>
> From 518b0cf9433914f437631f16816cfb564fcce285 Mon Sep 17 00:00:00 2001
> From: Fei Yang 
> Date: Mon, 27 Apr 2020 15:04:57 +0800
> Subject: [PATCH] forwprop: Fix ICE when building an identity constructor
>  [PR94784]
>
> In the testcase for PR94784, we have two vectors with the same ABI identity
> but with different TYPE_MODEs. It would be better to flip the assert around
> so that it checks that the two vectors have equal TYPE_VECTOR_SUBPARTS and
> that converting the corresponding element types is a 
> useless_type_conversion_p.
>
> 2020-04-27  Felix Yang  
>
> gcc/
> PR tree-optimization/94784
> * tree-ssa-forwprop.c (simplify_vector_constructor): Flip the
> assert around so that it checks that the two vectors have equal
> TYPE_VECTOR_SUBPARTS and that converting the corresponding element
> types is a useless_type_conversion_p.
>
> gcc/testsuite/
> PR tree-optimization/94784
> * gcc.dg/pr94784.c: New test.
> ---
>  gcc/ChangeLog  | 11 +--
>  gcc/testsuite/ChangeLog|  5 +
>  gcc/testsuite/gcc.dg/pr94784.c | 16 
>  gcc/tree-ssa-forwprop.c| 14 --
>  4 files changed, 42 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/pr94784.c
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 38dd5837e20..03f73c736ff 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,11 @@
> +2020-04-27  Felix Yang  
> +
> + PR tree-optimization/94784
> + * tree-ssa-forwprop.c (simplify_vector_constructor): Flip the
> + assert around so that it checks that the two vectors have equal
> + TYPE_VECTOR_SUBPARTS and that converting the corresponding element
> + types is a useless_type_conversion_p.
> +
>  2020-04-25  David Edelsohn  
>  
>   * config/rs6000/rs6000-logue.c (rs6000_stack_info): Don't push a
> @@ -1790,8 +1798,7 @@
>  
>   PR tree-optimization/94269
>   * tree-ssa-math-opts.c (convert_plusminus_to_widen): Restrict
> - this
> - operation to single basic block.
> + this operation to single basic block.

Not wrong, but might as well leave the entry as-is.

>  
>  2020-03-25  Jeff Law  
>  
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index 07fe8a68598..cfc7b13985d 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2020-04-27  Felix Yang  
> +
> + PR tree-optimization/94784
> + * gcc.dg/pr94784.c: New test.
> +
>  2020-04-26  Iain Sandoe  
>  
>   PR c++/94752
> diff --git a/gcc/testsuite/gcc.dg/pr94784.c b/gcc/testsuite/gcc.dg/pr94784.c
> new file mode 100644
> index 000..df6972f64aa
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr94784.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile { target aarch64*-*-* } } */
> +/* { dg-options "-O2 -ftree-slp-vectorize -march=armv8.2-a+sve 
> -msve-vector-bits=256" } */
> +
> +typedef short __attribute__((vector_size (8))) v4hi;
> +
> +typedef union U4HI { v4hi v; short a[4]; } u4hi;
> +
> +short a[4];
> +
> +void pass_v4hi (v4hi v) {
> +int j;
> +u4hi u;
> +u.v = v;
> +for (j = 0; j < 4; j++)
> +  a[j] = u.a[j];
> +};
> diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
> index 8ee5450b94c..7cfb18bd618 100644
> --- a/gcc/tree-ssa-forwprop.c
> +++ b/gcc/tree-ssa-forwprop.c
> @@ -2479,7 +2479,12 @@ simplify_vector_constructor (gimple_stmt_iterator *gsi)
> tree src_type = TREE_TYPE (orig[0]);
> if (!useless_type_conversion_p (type, src_type))
>   {
> -   gcc_assert (!targetm.compatible_vector_types_p (type, src_type));
> +   gcc_assert (TYPE_VECTOR_SUBPARTS (type).is_constant ()
> +   && TYPE_VECTOR_SUBPARTS (src_type).is_constant ()
> +   && (TYPE_VECTOR_SUBPARTS (type).to_constant ()
> +   == TYPE_VECTOR_SUBPARTS (src_type).to_constant ())

We don't need to fall back to constant comparisons here.
We should assert for known_eq between the TYPE_VECTOR_SUBPARTS instead.

Same for the other assert.

LGTM otherwise, and sorry for the breakage.

Thanks,
Richard