Message ID | b603d1dfc5122930728b042184295c490a35675c.camel@vnet.ibm.com |
---|---|
State | New |
Headers | show |
Series | [rs6000,gcc-8] Improve handling of built-in initialization. [PR95952] | expand |
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
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; }