* [PATCH, rs6000, gcc-8 ] Improve handling of built-in initialization. [PR95952]
@ 2020-07-14 17:15 will schmidt
2020-07-15 21:38 ` Segher Boessenkool
0 siblings, 1 reply; 2+ messages in thread
From: will schmidt @ 2020-07-14 17:15 UTC (permalink / raw)
To: GCC Patches
Cc: Peter Bergner, Segher Boessenkool, David Edelsohn, Bill Schmidt,
mikpelinux
Hi,
We've got a scenario with a combination of old hardware, gcc-8 and
binutils where gcc will ICE during it's selftest. This ICE was exposed
when the builtin processing for better #pragma support was added, where
we no longer skip builtin initialization based on the current mask.
Per the bug report and assorted debug, the ICE occurrs when building
the gcc-8 branch on a 970* based system with an old binutils. (gcc-9 and
newer is OK. binutils 2.34 is reported to allow success).
The attached patch adds a clause to the builtin initialization to skip
initialization of a builtin when the builtin mask is set but the icode
value is zero. The subsequent assert check remains in place.
I've successfully tested this on a yellowdog (970mp) based system.
Mikael has confirmed this allows success on his system.
OK for gcc-8 ?
Thanks,
-Will
2020-07-13 Will Schmidt <will_schmidt@vnet.ibm.com>
gcc/ChangeLog:
PR target/95952
* gcc/config/rs6000.c (altivec_init_builtins): Add continue clause to predicate
builtin handling.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 75d40367a98..18713599d3b 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -18015,10 +18015,24 @@ altivec_init_builtins (void)
if (rs6000_overloaded_builtin_p (d->code))
mode1 = VOIDmode;
else
{
+ /* PR95952: Gracefully skip builtins that do not have the icode properly
+ set, but do have the builtin mask set. This has occurred in older gcc
+ builds with older binutils support when binutils refuses code generation
+ for instructions that it does not support. This was exposed by changes
+ allowing all builtins being initialized for better #pragma support. */
+ if (d->icode == CODE_FOR_nothing && d->mask) {
+ HOST_WIDE_INT builtin_mask = rs6000_builtin_mask;
+ if (TARGET_DEBUG_BUILTIN) {
+ fprintf (stderr, "altivec_init_builtins, altivec predicate builtin %s", d->name);
+ fprintf (stderr, " was skipped. icode:%d, mask: %lx, builtin_mask: 0x%lx",
+ d->icode, d->mask, builtin_mask);
+ }
+ continue;
+ }
/* Cannot define builtin if the instruction is disabled. */
gcc_assert (d->icode != CODE_FOR_nothing);
mode1 = insn_data[d->icode].operand[1].mode;
}
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH, rs6000, gcc-8 ] Improve handling of built-in initialization. [PR95952]
2020-07-14 17:15 [PATCH, rs6000, gcc-8 ] Improve handling of built-in initialization. [PR95952] will schmidt
@ 2020-07-15 21:38 ` Segher Boessenkool
0 siblings, 0 replies; 2+ messages in thread
From: Segher Boessenkool @ 2020-07-15 21:38 UTC (permalink / raw)
To: will schmidt
Cc: GCC Patches, Peter Bergner, David Edelsohn, Bill Schmidt, mikpelinux
Hi!
On Tue, Jul 14, 2020 at 12:15:01PM -0500, will schmidt wrote:
> We've got a scenario with a combination of old hardware, gcc-8 and
> binutils where gcc will ICE during it's selftest. This ICE was exposed
> when the builtin processing for better #pragma support was added, where
> we no longer skip builtin initialization based on the current mask.
> OK for gcc-8 ?
Yes, but some formatting nits:
> + /* PR95952: Gracefully skip builtins that do not have the icode properly
> + set, but do have the builtin mask set. This has occurred in older gcc
> + builds with older binutils support when binutils refuses code generation
> + for instructions that it does not support. This was exposed by changes
> + allowing all builtins being initialized for better #pragma support. */
Nice useful comment :-)
> + if (d->icode == CODE_FOR_nothing && d->mask) {
> + HOST_WIDE_INT builtin_mask = rs6000_builtin_mask;
The { goes on the next line:
if (d->icode == CODE_FOR_nothing && d->mask)
{
HOST_WIDE_INT builtin_mask = rs6000_builtin_mask;
(two spaces indent, twice).
if (TARGET_DEBUG_BUILTIN)
{
fprintf (stderr, "altivec_init_builtins, altivec predicate builtin %s", d->name);
fprintf (stderr, " was skipped. icode:%d, mask: %lx, builtin_mask: 0x%lx",
d->icode, d->mask, builtin_mask);
(those lines are much too long, but debug code, I can't say I care much).
}
continue;
}
So: { always goes on a line of its own, two columns extra indent both
before and after it; } always aligns exactly with the {.
Okay for GCC 8 with that cleaned up. Thank you!
Segher
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-07-15 21:38 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-14 17:15 [PATCH, rs6000, gcc-8 ] Improve handling of built-in initialization. [PR95952] will schmidt
2020-07-15 21:38 ` Segher Boessenkool
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).