[Bug tree-optimization/105374] [12 Regression] ICE in fold_convert_loc, at fold-const.cc:2580 during GIMPLE pass: reassoc since r12-7338-g884f77b4222289510e1df9db2889b60c5df6fcda
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105374 --- Comment #11 from CVS Commits --- The master branch has been updated by Christophe Lyon : https://gcc.gnu.org/g:6c211e70eb9d89db2be630c0dd21e4aee144 commit r12-8279-g6c211e70eb9d89db2be630c0dd21e4aee144 Author: Christophe Lyon Date: Tue Apr 26 15:57:02 2022 +0100 testsuite: Add arm testcase for PR105374 As discussed in the PR, here is the testcase with the appropriate dg-* directives. Tested on arm-none-eabi with 1 -mcpu=cortex-a7/-mfloat-abi=soft/-march=armv7ve+simd 2 -mcpu=cortex-a7/-mfloat-abi=hard/-march=armv7ve+simd 3 -mthumb/-mcpu=cortex-a7/-mfloat-abi=hard/-march=armv7ve+simd 4 -mthumb/-mfloat-abi=soft/-march=armv6s-m 5 -mthumb/-mfloat-abi=soft/-march=armv7-m 6 -mthumb/-mfloat-abi=hard/-march=armv7e-m+fp 7 -mthumb/-mfloat-abi=hard/-march=armv7e-m+fp.dp 8 -mthumb/-mfloat-abi=hard/-march=armv8-m.main+fp+dsp 9 -mthumb/-mfloat-abi=hard/-march=armv8.1-m.main+mve.fp+fp.dp 10 -mthumb/-mfloat-abi=hard/-march=armv8.1-m.main+mve The test is UNSUPPORTED with the first three ones (because of -mcpu=cortex-a7), ignored with armv6s-m, and PASSes with all the other ones, while it used crash without Jakub's fix (r12-8263), ie. FAIL with options 5,6,7,8,10. The test passed without Jakub's fix with option 9 because the problem happens only with an integer-only MVE. 2022-04-26 Christophe Lyon gcc/testsuite/ PR tree-optimization/105374 * gcc.target/arm/simd/pr105374.C: New.
[Bug tree-optimization/105374] [12 Regression] ICE in fold_convert_loc, at fold-const.cc:2580 during GIMPLE pass: reassoc since r12-7338-g884f77b4222289510e1df9db2889b60c5df6fcda
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105374 Jakub Jelinek changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #10 from Jakub Jelinek --- Fixed.
[Bug tree-optimization/105374] [12 Regression] ICE in fold_convert_loc, at fold-const.cc:2580 during GIMPLE pass: reassoc since r12-7338-g884f77b4222289510e1df9db2889b60c5df6fcda
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105374 --- Comment #9 from CVS Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:d4836ac9acd0c991a4fe1dec9438773a2c4eb5ac commit r12-8263-gd4836ac9acd0c991a4fe1dec9438773a2c4eb5ac Author: Jakub Jelinek Date: Tue Apr 26 09:57:34 2022 +0200 reassoc: Don't call fold_convert if !fold_convertible_p [PR105374] 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. 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 PR tree-optimization/105374 * tree-ssa-reassoc.cc (eliminate_redundant_comparison): Punt if !fold_convertible_p rather than assuming fold_convert must succeed.
[Bug tree-optimization/105374] [12 Regression] ICE in fold_convert_loc, at fold-const.cc:2580 during GIMPLE pass: reassoc since r12-7338-g884f77b4222289510e1df9db2889b60c5df6fcda
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105374
--- Comment #8 from Richard Biener ---
(In reply to Jakub Jelinek from comment #2)
> This can (and IMHO should no matter what) be fixed in reassoc by:
> --- gcc/tree-ssa-reassoc.cc.jj2022-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);
> + }
Note for vectors the "appropriate" way to make a value from a bool is
bool ? -1 : 0
though I guess the question is whether we know if we want a vector true (-1)
or a scalar true (1) here ...
>
>if (TREE_CODE (t) != INTEGER_CST
> && !operand_equal_p (t, curr->op, 0))
>
> But another question is if we shouldn't actually optimize it rather than
> punting out.
Ideally yes.
> The reason why that happens is that while eliminate_redundant_comparison
> indirectly passes the V4BImode vector type as TYPE argument, the function
> doesn't actually use it and uses truth_type (V4SImode) instead.
> truth_type use has been introduced in r0-119133-gae22ac3c62db451bae
> but at that point the type argument didn't exist.
> That has been only introduced in r10-3154-g5f487a349de62613d7fa429 .
> I wonder if we can't just kill the truth_type computation and replace all
> uses with type...
I guess so - but does that fix anything here?
[Bug tree-optimization/105374] [12 Regression] ICE in fold_convert_loc, at fold-const.cc:2580 during GIMPLE pass: reassoc since r12-7338-g884f77b4222289510e1df9db2889b60c5df6fcda
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105374 --- Comment #7 from Martin Liška --- (In reply to Martin Liška from comment #6) > Btw. started with r12-7338-g884f77b489 if that helps. Oh, it was already discovered by Jakub.
[Bug tree-optimization/105374] [12 Regression] ICE in fold_convert_loc, at fold-const.cc:2580 during GIMPLE pass: reassoc since r12-7338-g884f77b4222289510e1df9db2889b60c5df6fcda
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105374 Martin Liška changed: What|Removed |Added Last reconfirmed||2022-04-25 Ever confirmed|0 |1 Status|UNCONFIRMED |NEW CC||marxin at gcc dot gnu.org --- Comment #6 from Martin Liška --- Btw. started with r12-7338-g884f77b489 if that helps.
[Bug tree-optimization/105374] [12 Regression] ICE in fold_convert_loc, at fold-const.cc:2580 during GIMPLE pass: reassoc since r12-7338-g884f77b4222289510e1df9db2889b60c5df6fcda
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105374 --- Comment #5 from Christophe Lyon --- > Regarding the dg-* directives, I suspect you need arm_v8_1m_mve_fp_ok since > the test involves floats. I was wrong and your proposal of arm_v8_1m_mve_ok looks fine (since actually there is no ICE with compiling for an FP-capable FPU)
[Bug tree-optimization/105374] [12 Regression] ICE in fold_convert_loc, at fold-const.cc:2580 during GIMPLE pass: reassoc since r12-7338-g884f77b4222289510e1df9db2889b60c5df6fcda
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105374 --- Comment #4 from Christophe Lyon --- Other MVE tests are in gcc.target/arm/simd/ (eg mve-vcmp-f32.c), maybe it's best to keep them in the same place? Regarding the dg-* directives, I suspect you need arm_v8_1m_mve_fp_ok since the test involves floats. Thanks for the prompt fix!
[Bug tree-optimization/105374] [12 Regression] ICE in fold_convert_loc, at fold-const.cc:2580 during GIMPLE pass: reassoc since r12-7338-g884f77b4222289510e1df9db2889b60c5df6fcda
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105374
--- Comment #3 from Jakub Jelinek ---
Also, no idea where exactly to put the testcase to and what dg-* directives to
use, arm testcases is something I'm really not familiar with.
Perhaps gcc.target/arm/mve/general
and
/* { dg-do compile } */
/* { dg-require-effective-target arm_v8_1m_mve_ok } */
/* { dg-add-options arm_v8_1m_mve } */
but can -mfloat-abi=hard be just added or does it require some extra magic?
[Bug tree-optimization/105374] [12 Regression] ICE in fold_convert_loc, at fold-const.cc:2580 during GIMPLE pass: reassoc since r12-7338-g884f77b4222289510e1df9db2889b60c5df6fcda
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105374 Jakub Jelinek changed: What|Removed |Added Target Milestone|--- |12.0 Priority|P3 |P1
[Bug tree-optimization/105374] [12 Regression] ICE in fold_convert_loc, at fold-const.cc:2580 during GIMPLE pass: reassoc since r12-7338-g884f77b4222289510e1df9db2889b60c5df6fcda
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105374
Jakub Jelinek changed:
What|Removed |Added
CC||jakub at gcc dot gnu.org
--- Comment #2 from Jakub Jelinek ---
This can (and IMHO should no matter what) be fixed in reassoc by:
--- 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))
But another question is if we shouldn't actually optimize it rather than
punting out.
The reason why that happens is that while eliminate_redundant_comparison
indirectly passes the V4BImode vector type as TYPE argument, the function
doesn't actually use it and uses truth_type (V4SImode) instead.
truth_type use has been introduced in r0-119133-gae22ac3c62db451bae
but at that point the type argument didn't exist.
That has been only introduced in r10-3154-g5f487a349de62613d7fa429 .
I wonder if we can't just kill the truth_type computation and replace all uses
with type...
[Bug tree-optimization/105374] [12 Regression] ICE in fold_convert_loc, at fold-const.cc:2580 during GIMPLE pass: reassoc since r12-7338-g884f77b4222289510e1df9db2889b60c5df6fcda
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105374 Alex Coplan changed: What|Removed |Added Known to fail||12.0 Known to work||11.3.1 Keywords||ice-on-valid-code Target||arm-eabi --- Comment #1 from Alex Coplan --- I should have mentioned the testcase is reduced from g++.dg/other/pr95231.C
