Created attachment 56862 [details]
Proposed patch

Patch in testing.

Started with r14-1145-g1ede03e2d0437ea9c2f7453fcbe263505b4e0def

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.

(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.

With -O1 or higher there is some DCE which will remove them (unless disabled), but the above ICE is with -O0...

(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--

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
11098	     if there is a side-effect somewhere in the expression.  If there
11099	     is, short-circuit the most common cases here.  Note that we must
11100	     not call expand_expr with anything but const0_rtx in case this
11101	     is 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...

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).

(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.

(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?

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...

(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.

> 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.

Created attachment 56863 [details]
gcc14-pr112962-1.patch

Patch I'm going to test.

Created attachment 56864 [details]
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.

(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.

The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:02c30fdad2f46a1f7b4e30d0eff0ac275cd108a5

commit r14-6485-g02c30fdad2f46a1f7b4e30d0eff0ac275cd108a5
Author: Jakub Jelinek <jakub@redhat.com>
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  <jakub@redhat.com>
    
            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.

Fixed.

The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:96e0b513717e25405aee36851d5164aab0d0403a

commit r14-6743-g96e0b513717e25405aee36851d5164aab0d0403a
Author: Jakub Jelinek <jakub@redhat.com>
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  <jakub@redhat.com>
    
            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.