[PATCH] reassoc: Don't call fold_convert if !fold_convertible_p [PR105374]

[PATCH] reassoc: Don't call fold_convert if !fold_convertible_p [PR105374]

[Jakub Jelinek]

Hi!

As mentioned in the PR, we ICE because maybe_fold_*_comparisons returns
an expression with V4SImode type and we try to fold_convert it to
V4BImode, which isn't allowed.

IMHO no matter whether we change maybe_fold_*_comparisons we should
play safe on the reassoc side and punt if we can't convert like
we punt for many other reasons.  This fixes the testcase on ARM.

Bootstrapped/regtested on {x86_64,i686,armv7hl,aarch64,powerpc64le}-linux,
ok for trunk?

Testcase not included, not exactly sure where and what directives it
should have in gcc.target/arm/ testsuite.  Christophe, do you think you
could handle that incrementally?

2022-04-26  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/105374
	* tree-ssa-reassoc.cc (eliminate_redundant_comparison): Punt if
	!fold_convertible_p rather than assuming fold_convert must succeed.

--- gcc/tree-ssa-reassoc.cc.jj	2022-04-14 13:46:59.690140053 +0200
+++ gcc/tree-ssa-reassoc.cc	2022-04-25 15:34:03.811473537 +0200
@@ -2254,7 +2254,11 @@ eliminate_redundant_comparison (enum tre
 	 BIT_AND_EXPR or BIT_IOR_EXPR was of a wider integer type,
 	 we need to convert.  */
       if (!useless_type_conversion_p (TREE_TYPE (curr->op), TREE_TYPE (t)))
-	t = fold_convert (TREE_TYPE (curr->op), t);
+	{
+	  if (!fold_convertible_p (TREE_TYPE (curr->op), t))
+	    continue;
+	  t = fold_convert (TREE_TYPE (curr->op), t);
+	}
 
       if (TREE_CODE (t) != INTEGER_CST
 	  && !operand_equal_p (t, curr->op, 0))

	Jakub

Re: [PATCH] reassoc: Don't call fold_convert if !fold_convertible_p [PR105374]

[Richard Biener]

On Tue, 26 Apr 2022, Jakub Jelinek wrote:

> Hi!
> 
> As mentioned in the PR, we ICE because maybe_fold_*_comparisons returns
> an expression with V4SImode type and we try to fold_convert it to
> V4BImode, which isn't allowed.
> 
> IMHO no matter whether we change maybe_fold_*_comparisons we should
> play safe on the reassoc side and punt if we can't convert like
> we punt for many other reasons.  This fixes the testcase on ARM.
> 
> Bootstrapped/regtested on {x86_64,i686,armv7hl,aarch64,powerpc64le}-linux,
> ok for trunk?

OK.

Richard.

> Testcase not included, not exactly sure where and what directives it
> should have in gcc.target/arm/ testsuite.  Christophe, do you think you
> could handle that incrementally?
> 
> 2022-04-26  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/105374
> 	* tree-ssa-reassoc.cc (eliminate_redundant_comparison): Punt if
> 	!fold_convertible_p rather than assuming fold_convert must succeed.
> 
> --- gcc/tree-ssa-reassoc.cc.jj	2022-04-14 13:46:59.690140053 +0200
> +++ gcc/tree-ssa-reassoc.cc	2022-04-25 15:34:03.811473537 +0200
> @@ -2254,7 +2254,11 @@ eliminate_redundant_comparison (enum tre
>  	 BIT_AND_EXPR or BIT_IOR_EXPR was of a wider integer type,
>  	 we need to convert.  */
>        if (!useless_type_conversion_p (TREE_TYPE (curr->op), TREE_TYPE (t)))
> -	t = fold_convert (TREE_TYPE (curr->op), t);
> +	{
> +	  if (!fold_convertible_p (TREE_TYPE (curr->op), t))
> +	    continue;
> +	  t = fold_convert (TREE_TYPE (curr->op), t);
> +	}
>  
>        if (TREE_CODE (t) != INTEGER_CST
>  	  && !operand_equal_p (t, curr->op, 0))
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)

Re: [PATCH] reassoc: Don't call fold_convert if !fold_convertible_p [PR105374]

[Christophe Lyon]

On 4/26/22 09:24, Jakub Jelinek wrote:
> Hi!
> 
> As mentioned in the PR, we ICE because maybe_fold_*_comparisons returns
> an expression with V4SImode type and we try to fold_convert it to
> V4BImode, which isn't allowed.
> 
> IMHO no matter whether we change maybe_fold_*_comparisons we should
> play safe on the reassoc side and punt if we can't convert like
> we punt for many other reasons.  This fixes the testcase on ARM.
> 
> Bootstrapped/regtested on {x86_64,i686,armv7hl,aarch64,powerpc64le}-linux,
> ok for trunk?
> 
> Testcase not included, not exactly sure where and what directives it
> should have in gcc.target/arm/ testsuite.  Christophe, do you think you
> could handle that incrementally?

Yes, sure, I'll take a look.
Thanks for the fix!

> 
> 2022-04-26  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/105374
> 	* tree-ssa-reassoc.cc (eliminate_redundant_comparison): Punt if
> 	!fold_convertible_p rather than assuming fold_convert must succeed.
> 
> --- gcc/tree-ssa-reassoc.cc.jj	2022-04-14 13:46:59.690140053 +0200
> +++ gcc/tree-ssa-reassoc.cc	2022-04-25 15:34:03.811473537 +0200
> @@ -2254,7 +2254,11 @@ eliminate_redundant_comparison (enum tre
>   	 BIT_AND_EXPR or BIT_IOR_EXPR was of a wider integer type,
>   	 we need to convert.  */
>         if (!useless_type_conversion_p (TREE_TYPE (curr->op), TREE_TYPE (t)))
> -	t = fold_convert (TREE_TYPE (curr->op), t);
> +	{
> +	  if (!fold_convertible_p (TREE_TYPE (curr->op), t))
> +	    continue;
> +	  t = fold_convert (TREE_TYPE (curr->op), t);
> +	}
>   
>         if (TREE_CODE (t) != INTEGER_CST
>   	  && !operand_equal_p (t, curr->op, 0))
> 
> 	Jakub
>