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

2021-12-13 Thread marxin at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

Martin Liška  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

2021-12-13 Thread wschmidt at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #1 from Bill Schmidt  ---
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

2021-12-14 Thread bergner at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #2 from Peter Bergner  ---
(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

2021-12-14 Thread bergner at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #3 from Peter Bergner  ---
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

2021-12-14 Thread wschmidt at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #4 from Bill Schmidt  ---
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

2021-12-14 Thread wschmidt at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #5 from Bill Schmidt  ---
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

2021-12-14 Thread wschmidt at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #6 from Bill Schmidt  ---
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

2022-01-04 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #7 from Richard Biener  ---
-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

2022-01-04 Thread segher at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #8 from Segher Boessenkool  ---
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

2022-01-04 Thread bergner at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

Peter Bergner  changed:

   What|Removed |Added

 CC||willschm at gcc dot gnu.org

--- Comment #9 from Peter Bergner  ---
(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

2022-01-19 Thread wschmidt at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #10 from Bill Schmidt  ---
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

2022-01-20 Thread willschm at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #11 from Will Schmidt  ---
(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

2022-02-03 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

--- Comment #12 from CVS Commits  ---
The master branch has been updated by William Schmidt :

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

commit r12-7030-g48bd780ee327c9ae6ffc0641e73cc1f4939fb204
Author: Bill Schmidt 
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  

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

2022-02-03 Thread wschmidt at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103686

Bill Schmidt  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|NEW |RESOLVED

--- Comment #13 from Bill Schmidt  ---
Fixed now.