[PATCH, rs6000, gcc-8 ] Improve handling of built-in initialization. [PR95952]

[PATCH, rs6000, gcc-8 ] Improve handling of built-in initialization. [PR95952]

[will schmidt]

    
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;
 	}
 

Re: [PATCH, rs6000, gcc-8 ] Improve handling of built-in initialization. [PR95952]

[Segher Boessenkool]

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