[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 --- Comment #19 from GCC Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:96e0b513717e25405aee36851d5164aab0d0403a commit r14-6743-g96e0b513717e25405aee36851d5164aab0d0403a Author: Jakub Jelinek Date: Wed Dec 20 12:01:57 2023 +0100 i386: Make most MD builtins nothrow, leaf [PR112962] The following patch makes most of x86 MD builtins nothrow,leaf (like most middle-end builtins are). For -fnon-call-exceptions it doesn't nothrow, better might be to still add it if the builtins don't read or write memory and can't raise floating point exceptions, but we don't have such information readily available, so the patch uses just !flag_non_call_exceptions for now. Not sure if we shouldn't have some exceptions for the leaf attribute, e.g. wonder about EMMS/FEMMS and the various xsave/xrstor etc. builtins, pedantically none of those builtins do anything that leaf functions are forbidden to do (having callbacks, calling functions from current TU, longjump into the current TU), but sometimes non-leaf is also used on really complex functions to prevent some unwanted optimizations. That said, haven't run into any problems as is with the patch. 2023-12-20 Jakub Jelinek PR target/112962 * config/i386/i386-builtins.cc (ix86_builtins): Increase by one element. (def_builtin): If not -fnon-call-exceptions, set TREE_NOTHROW on the builtin FUNCTION_DECL. Add leaf attribute to DECL_ATTRIBUTES. (ix86_add_new_builtins): Likewise.
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 Jakub Jelinek changed: What|Removed |Added Resolution|--- |FIXED Status|ASSIGNED|RESOLVED --- Comment #18 from Jakub Jelinek --- Fixed.
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 --- Comment #17 from GCC Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:02c30fdad2f46a1f7b4e30d0eff0ac275cd108a5 commit r14-6485-g02c30fdad2f46a1f7b4e30d0eff0ac275cd108a5 Author: Jakub Jelinek Date: Wed Dec 13 11:34:12 2023 +0100 i386: Fix ICE on __builtin_ia32_pabsd128 without lhs [PR112962] The following patch fixes ICE on the testcase in similar way to how other folded builtins are handled in ix86_gimple_fold_builtin when they don't have a lhs; these builtins are const or pure, so normally DCE would remove them later, but with -O0 that isn't guaranteed to happen, and during expansion if they are marked TREE_SIDE_EFFECTS it might still be attempted to be expanded. This removes them right away during the folding. Initially I wanted to also change all gsi_replace last args in that function to true, but Andrew pointed to PR107209, so I've kept them as is. 2023-12-13 Jakub Jelinek PR target/112962 * config/i386/i386.cc (ix86_gimple_fold_builtin): For shifts and abs without lhs replace with nop. * gcc.target/i386/pr112962.c: New test.
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 --- Comment #16 from Andrew Pinski --- (In reply to Jakub Jelinek from comment #15) > I think e.g. aarch64 doesn't set nothrow on builtins with > -fnon-call-exceptions if they might raise floating point exceptions or > read/write memory. That is correct, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107209#c7 for a change related to aarch64's builtins and non-call exceptions and folding there.
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 --- Comment #15 from Jakub Jelinek --- Created attachment 56864 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56864=edit gcc14-pr112962-2.patch And another one for nothrow/leaf. I'm too lazy to figure out the exact details for -fnon-call-exceptions, will defer that to somebody who cares about those and has time to figure out all the details. I think e.g. aarch64 doesn't set nothrow on builtins with -fnon-call-exceptions if they might raise floating point exceptions or read/write memory.
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 Jakub Jelinek changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org --- Comment #14 from Jakub Jelinek --- Created attachment 56863 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56863=edit gcc14-pr112962-1.patch Patch I'm going to test.
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 --- Comment #13 from Hongtao Liu --- > I prefer this solution, that's what we did for blendvps case. > I don't know either, just follow what we did before (with false) when > folding builtins. I mean when I was working on r14-1145-g1ede03e2d0437ea9c2f7453fcbe263505b4e0def, I'm just follow the existed code.
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962
--- Comment #12 from Hongtao Liu ---
(In reply to Jakub Jelinek from comment #8)
> Of course, yet another option is:
> --- gcc/config/i386/i386.cc 2023-12-12 08:54:39.821148670 +0100
> +++ gcc/config/i386/i386.cc 2023-12-12 11:07:03.795286363 +0100
> @@ -19377,7 +19377,10 @@ ix86_gimple_fold_builtin (gimple_stmt_it
> do_shift:
>gcc_assert (n_args >= 2);
>if (!gimple_call_lhs (stmt))
> - break;
> + {
> + gsi_replace (gsi, gimple_build_nop (), false);
> + return true;
> + }
>arg0 = gimple_call_arg (stmt, 0);
>arg1 = gimple_call_arg (stmt, 1);
>elems = TYPE_VECTOR_SUBPARTS (TREE_TYPE (arg0));
> @@ -19523,7 +19526,10 @@ ix86_gimple_fold_builtin (gimple_stmt_it
> case IX86_BUILTIN_PABSD256_MASK:
>gcc_assert (n_args >= 1);
>if (!gimple_call_lhs (stmt))
> - break;
> + {
> + gsi_replace (gsi, gimple_build_nop (), false);
> + return true;
> + }
>arg0 = gimple_call_arg (stmt, 0);
>elems = TYPE_VECTOR_SUBPARTS (TREE_TYPE (arg0));
>/* For masked ABS, only optimize if the mask is all ones. */
> (and I wonder why all the gsi_replace calls in that function are with false,
> IMHO they should use true).
I prefer this solution, that's what we did for blendvps case.
I don't know either, just follow what we did before (with false) when folding
builtins.
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 --- Comment #11 from Jakub Jelinek --- Sure. But we need to find out first what builtins might actually throw. Perhaps with -fnon-call-exceptions those which read/store (vector loads/stores, masked loads/stores, scatters/gathers?) memory can? Unsure if we handle it though...
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 Uroš Bizjak changed: What|Removed |Added Assignee|ubizjak at gmail dot com |unassigned at gcc dot gnu.org Status|ASSIGNED|NEW --- Comment #10 from Uroš Bizjak --- (In reply to Jakub Jelinek from comment #7) > but with -fexceptions (and probably because we incorrectly don't mark the > builtins nothrow?) this doesn't happen. Maybe we should finally fix the above nothrow issue?
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 --- Comment #9 from Uroš Bizjak --- (In reply to Jakub Jelinek from comment #8) > Of course, yet another option is: This goes out of my (limited) area of expertise, so if my proposed (trivial) patch is papering over some other issue, I'll happily leave the solution to you.
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962
--- Comment #8 from Jakub Jelinek ---
Of course, yet another option is:
--- gcc/config/i386/i386.cc 2023-12-12 08:54:39.821148670 +0100
+++ gcc/config/i386/i386.cc 2023-12-12 11:07:03.795286363 +0100
@@ -19377,7 +19377,10 @@ ix86_gimple_fold_builtin (gimple_stmt_it
do_shift:
gcc_assert (n_args >= 2);
if (!gimple_call_lhs (stmt))
- break;
+ {
+ gsi_replace (gsi, gimple_build_nop (), false);
+ return true;
+ }
arg0 = gimple_call_arg (stmt, 0);
arg1 = gimple_call_arg (stmt, 1);
elems = TYPE_VECTOR_SUBPARTS (TREE_TYPE (arg0));
@@ -19523,7 +19526,10 @@ ix86_gimple_fold_builtin (gimple_stmt_it
case IX86_BUILTIN_PABSD256_MASK:
gcc_assert (n_args >= 1);
if (!gimple_call_lhs (stmt))
- break;
+ {
+ gsi_replace (gsi, gimple_build_nop (), false);
+ return true;
+ }
arg0 = gimple_call_arg (stmt, 0);
elems = TYPE_VECTOR_SUBPARTS (TREE_TYPE (arg0));
/* For masked ABS, only optimize if the mask is all ones. */
(and I wonder why all the gsi_replace calls in that function are with false,
IMHO they should use true).
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962
--- Comment #7 from Jakub Jelinek ---
On the other side, maybe we want some of the diagnostics that is only done at
expansion time (argument xyz must be such and such immediate). Though, at -O2
it isn't diagnosed anyway because it is DCEd.
Anyway, with just -O0 -mssse3 this works fine because of expr.cc:
11097 /* If we are going to ignore this result, we need only do something
11098if there is a side-effect somewhere in the expression. If there
11099is, short-circuit the most common cases here. Note that we must
11100not call expand_expr with anything but const0_rtx in case this
11101is an initial expansion of a size that contains a
PLACEHOLDER_EXPR. */
11102
11103 if (ignore)
11104 {
11105 if (! TREE_SIDE_EFFECTS (exp))
11106 return const0_rtx;
but with -fexceptions (and probably because we incorrectly don't mark the
builtins nothrow?) this doesn't happen.
I was thinking about something like
--- gcc/i386-expand.cc.jj 2023-12-07 08:31:59.855850982 +0100
+++ gcc/i386-expand.cc 2023-12-12 11:02:54.524733315 +0100
@@ -11842,6 +11842,12 @@ ix86_expand_args_builtin (const struct b
xops[i] = op;
}
+ if (icode == CODE_FOR_nothing)
+{
+ gcc_assert (target == const0_rtx);
+ return const0_rtx;
+}
+
switch (nargs)
{
case 1:
but of course, your choice...
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962
--- Comment #6 from Uroš Bizjak ---
(In reply to Jakub Jelinek from comment #3)
> I was thinking whether it wouldn't be better to expand x86 const or pure
> builtins when lhs is ignored to nothing in the expanders.
Something like this?
--cut here--
diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc
index a53d69d5400..0f3d6108d77 100644
--- a/gcc/config/i386/i386-expand.cc
+++ b/gcc/config/i386/i386-expand.cc
@@ -13032,6 +13032,9 @@ ix86_expand_builtin (tree exp, rtx target, rtx
subtarget,
unsigned int fcode = DECL_MD_FUNCTION_CODE (fndecl);
HOST_WIDE_INT bisa, bisa2;
+ if (ignore && (TREE_READONLY (fndecl) || DECL_PURE_P (fndecl)))
+return const0_rtx;
+
/* For CPU builtins that can be folded, fold first and expand the fold. */
switch (fcode)
{
@@ -14401,9 +14404,6 @@ rdseed_step:
return target;
case IX86_BUILTIN_READ_FLAGS:
- if (ignore)
- return const0_rtx;
-
emit_insn (gen_pushfl ());
if (optimize
--cut here--
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 --- Comment #5 from Jakub Jelinek --- With -O1 or higher there is some DCE which will remove them (unless disabled), but the above ICE is with -O0...
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 --- Comment #4 from Uroš Bizjak --- (In reply to Jakub Jelinek from comment #3) > I was thinking whether it wouldn't be better to expand x86 const or pure > builtins when lhs is ignored to nothing in the expanders. Yes, this could be a better solution.
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #3 from Jakub Jelinek --- I was thinking whether it wouldn't be better to expand x86 const or pure builtins when lhs is ignored to nothing in the expanders.
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 --- Comment #2 from Jakub Jelinek --- Started with r14-1145-g1ede03e2d0437ea9c2f7453fcbe263505b4e0def
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 Uroš Bizjak changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |ubizjak at gmail dot com Last reconfirmed||2023-12-12 Status|UNCONFIRMED |ASSIGNED Ever confirmed|0 |1 --- Comment #1 from Uroš Bizjak --- Created attachment 56862 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56862=edit Proposed patch Patch in testing.
[Bug target/112962] [14 Regression] ICE: SIGSEGV in operator() (recog.h:431) with -fexceptions -mssse3 and __builtin_ia32_pabsd128()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112962 Richard Biener changed: What|Removed |Added Target Milestone|--- |14.0
