[Bug target/103686] New: ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[Bug target/103686] New: ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

            Bug ID: 103686
           Summary: ICE in rs6000_expand_new_builtin at
                    rs6000-call.c:15946
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: ice-on-invalid-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: marxin at gcc dot gnu.org
  Target Milestone: ---
              Host: x86_64-linux-gnu
            Target: ppc64-linux-gnu

The following crashes:

$ ppc64-linux-gnu-gcc
/home/marxin/Programming/gcc/gcc/testsuite/gcc.target/powerpc/pr102347.c -c
-mno-fold-gimple
gimple folding of rs6000 builtins has been disabled.
gimple folding of rs6000 builtins has been disabled.
gimple folding of rs6000 builtins has been disabled.
gimple folding of rs6000 builtins has been disabled.
gimple folding of rs6000 builtins has been disabled.
gimple folding of rs6000 builtins has been disabled.
gimple folding of rs6000 builtins has been disabled.
during RTL pass: expand
/home/marxin/Programming/gcc/gcc/testsuite/gcc.target/powerpc/pr102347.c: In
function ‘main’:
/home/marxin/Programming/gcc/gcc/testsuite/gcc.target/powerpc/pr102347.c:13:3:
internal compiler error: Segmentation fault
   13 |   __builtin_mma_disassemble_acc (b, &c);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0xb6e79f crash_signal
       
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-trunk-ppc64/build/gcc/toplev.c:322
0x7ffff787d42f ???
       
/usr/src/debug/glibc-2.34-4.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0

[Bug target/103686] ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |segher at gcc dot gnu.org
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-12-13
             Status|UNCONFIRMED                 |NEW

[Bug target/103686] ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[wschmidt at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #1 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
I think that the MMA implementation is incompatible with -mno-fold-gimple. 
We'll need to prevent that flag combination, I think.

[Bug target/103686] ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #2 from Peter Bergner <bergner at gcc dot gnu.org> ---
(In reply to Bill Schmidt from comment #1)
> I think that the MMA implementation is incompatible with -mno-fold-gimple. 
> We'll need to prevent that flag combination, I think.

Does -mno-fold-gimple really even make sense as an option?  I agree that the
MMA builtins require gimple folding of builtins.  One other option is to just
ignore rs6000_fold_gimple (true or false) if the builtin attribute has
RS6000_BTC_GIMPLE set.

[Bug target/103686] ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #3 from Peter Bergner <bergner at gcc dot gnu.org> ---
Maybe something like this untested patch:

diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index d9736eaf21c..c7babefa32d 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -13615,12 +13615,14 @@ rs6000_gimple_fold_new_builtin (gimple_stmt_iterator
*gsi)
   const char *fn_name2 = (icode != CODE_FOR_nothing)
                          ? get_insn_name ((int) icode)
                          : "nothing";
+  unsigned attr = rs6000_builtin_info[fn_code].attr;

   if (TARGET_DEBUG_BUILTIN)
       fprintf (stderr, "rs6000_gimple_fold_new_builtin %d %s %s\n",
               fn_code, fn_name1, fn_name2);

-  if (!rs6000_fold_gimple)
+  if ((attr & RS6000_BTC_GIMPLE) == 0
+      && !rs6000_fold_gimple)
     return false;

   /* Prevent gimple folding for code that does not have a LHS, unless it is

Same thing needs to happen to rs6000_gimple_fold_builtin too

[Bug target/103686] ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[wschmidt at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #4 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
Please don't make changes to the old builtin support, which has been disabled.
:-)

[Bug target/103686] ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[wschmidt at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #5 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
More properly, please don't rely on a bit that is being destroyed by the new
support.  You need to look at built-in function attributes instead.

[Bug target/103686] ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[wschmidt at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #6 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
if (bif_is_mmaint (rs6000_builtin_info_x[uns_fcode])
    && !rs6000_fold_gimple)

is what you're looking for.  However, I would much rather see rejection of the
-mno-fold-gimple flag when MMA is enabled.  Silently ignoring a flag violates
the principle of least astonishment.

[Bug target/103686] ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
-m[no-]fold-gimple is also a very badly named user option since it suggests
that 'fold' and 'gimple' are terms known to programmers.  I'm just guessing
it was added to avoid "inlining" intrinsics as GIMPLE, so
-m[no-]inline-intrinsics would have been more appropriate ;)  (or
-m[no-]optimize-intrinsics)

[Bug target/103686] ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #8 from Segher Boessenkool <segher at gcc dot gnu.org> ---
It is an internal (debugging) option.  It isn't documented in the manual, but
indeed it is not marked as Undocumented in rs6000.opt .

[Bug target/103686] ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

Peter Bergner <bergner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |willschm at gcc dot gnu.org

--- Comment #9 from Peter Bergner <bergner at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #8)
> It is an internal (debugging) option.  It isn't documented in the manual, but
> indeed it is not marked as Undocumented in rs6000.opt .

Will added this and the last time we talked, he said he added it mainly for
debugging purposes when he was adding some rs6000 specific gimple builtin
expansion code.  To his knowledge, we don't need/want this anymore, so I
believe the correct fix here is to just remove the option now (assuming it
really is undocumented).

[Bug target/103686] ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[wschmidt at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #10 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
It turns out not to be undocumented -- but I'd like to remove it anyway.  Any
objections?

[Bug target/103686] ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[willschm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #11 from Will Schmidt <willschm at gcc dot gnu.org> ---
(In reply to Bill Schmidt from comment #10)
> It turns out not to be undocumented -- but I'd like to remove it anyway. 
> Any objections?

Realistically I believe I was the only user of that feature, was to help with
comparing the before/after codegen for the builtins as I added the gimple
folding logic for them.

No objections for removal of the option from me.

Knowing the history and the compromises involved, at this point I actually
encourage the removal of the option.  :-) 
Thanks,
-Will

[Bug target/103686] ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by William Schmidt <wschmidt@gcc.gnu.org>:

https://gcc.gnu.org/g:48bd780ee327c9ae6ffc0641e73cc1f4939fb204

commit r12-7030-g48bd780ee327c9ae6ffc0641e73cc1f4939fb204
Author: Bill Schmidt <wschmidt@linux.ibm.com>
Date:   Wed Feb 2 21:30:27 2022 -0600

    rs6000: Remove -m[no-]fold-gimple flag [PR103686]

    The -m[no-]fold-gimple flag was really intended primarily for internal
    testing while implementing GIMPLE folding for rs6000 vector built-in
    functions.  It ended up leaking into other places, causing problems such
    as PR103686 identifies.  Let's remove it.

    There are a number of tests in the testsuite that require adjustment.
    Some specify -mfold-gimple directly, which is the default, so that is
    handled by removing the option.  Others unnecessarily specify
    -mno-fold-gimple, as the tests work fine without this.  Again that is
    handled by removing the option.  There are a couple of extra variants of
    tests specifically for -mno-fold-gimple; for those, we can just remove the
    whole test.

    gcc.target/powerpc/builtins-1.c was more problematic.  It was written in
    such a way as to be extremely fragile.  For this one, I rewrote the whole
    test in a different style, using individual functions to test each
    built-in function.  These same tests are also largely covered by
    builtins-1-be-folded.c and builtins-1-le-folded.c, so I chose to
    explicitly make this test -mbig for simplicity, and use -O2 for clean code
    generation.  I made some slight modifications to the expected instruction
    counts as a result, and tested on both 32- and 64-bit.

    2022-02-02  Bill Schmidt  <wschmidt@linux.ibm.com>

    gcc/
            PR target/103686
            * config/rs6000/rs6000-builtin.cc (rs6000_gimple_fold_builtin):
Remove
            test for !rs6000_fold_gimple.
            * config/rs6000/rs6000.cc (rs6000_option_override_internal):
Likewise.
            * config/rs6000/rs6000.opt (mfold-gimple): Remove.

    gcc/testsuite/
            PR target/103686
            * gcc.target/powerpc/builtins-1-be-folded.c: Remove -mfold-gimple
            option.
            * gcc.target/powerpc/builtins-1-le-folded.c: Likewise.
            * gcc.target/powerpc/builtins-1.c: Rewrite to use small functions
and
            restrict to -O2 -mbig for predictability.  Adjust instruction
counts.
            * gcc.target/powerpc/builtins-5.c: Remove -mno-fold-gimple option.
            * gcc.target/powerpc/p8-vec-xl-xst.c: Likewise.
            * gcc.target/powerpc/pr83926.c: Likewise.
            * gcc.target/powerpc/pr86731-nogimplefold-longlong.c: Delete.
            * gcc.target/powerpc/pr86731-nogimplefold.c: Delete.
            * gcc.target/powerpc/swaps-p8-17.c: Remove -mno-fold-gimple option.

[Bug target/103686] ICE in rs6000_expand_new_builtin at rs6000-call.c:15946

[wschmidt at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

Bill Schmidt <wschmidt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #13 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
Fixed now.