Re: [PATCH] PR78056: Fix build failure on Power7

[Segher Boessenkool]

On Wed, Oct 26, 2016 at 08:57:11AM -0500, Bill Schmidt wrote:
> On Oct 26, 2016, at 8:29 AM, Segher Boessenkool  
> wrote:
> > 
> > So, you do not want to create the builtins that we expand to machine insns
> > that are not supported with the -mcpu= (or other flags) in use.  What does
> > the ABI have to say about this?
> 
> The ABI is silent on this point.  The appendix of builtin functions for 
> vector processing
> tags these as POWER ISA 3.0, but that's the extent of it.  I don't see a 
> problem with
> disabling built-ins that generate code that won't assemble provided we 
> diagnose the
> error properly (the follow-up work that Kelvin mentioned).

Okay, so let's do the sane thing then :-)

The patch is okay for trunk (with the changelog fixed).  Thanks Kelvin!


Segher

Re: [PATCH] PR78056: Fix build failure on Power7

[Bill Schmidt]

On Oct 26, 2016, at 8:29 AM, Segher Boessenkool  
wrote:
> 
> So, you do not want to create the builtins that we expand to machine insns
> that are not supported with the -mcpu= (or other flags) in use.  What does
> the ABI have to say about this?

The ABI is silent on this point.  The appendix of builtin functions for vector 
processing
tags these as POWER ISA 3.0, but that's the extent of it.  I don't see a 
problem with
disabling built-ins that generate code that won't assemble provided we diagnose 
the
error properly (the follow-up work that Kelvin mentioned).

I'll take any blame for suggesting that Kelvin go forward with a patch to get 
bootstrap
fixed and defer the error detection till afterwards...

Bill

Re: [PATCH] PR78056: Fix build failure on Power7

[Segher Boessenkool]

Hi Kelvin,

On Wed, Oct 26, 2016 at 02:41:43PM +0200, Segher Boessenkool wrote:
>   PR target/78056
>   * config/rs6000/rs6000.c (spe_init_builtins): Modify loops to not
>   define builtin functions from the bdesc_spe_predicates or
>   bdesc_spe_evsel arrays if the builtin mask is not compatible with
>   the current compiler configuration.
>   (paired_init_builtins): Modify loop to not define define builtin
>   functions from the bdesc_paried_preds array if the builtin mask is
>   not compatible with the current compiler configuration.

Why do you change the SPE and PS builtins init as well?  Were they buggy
in the same way, we just never noticed?

>   (altivec_init_builtins): Modify loops to not define the
>   __builtin_altivec_stxvl function nor the builtin functions from
>   the bdesc_dst or bdesc_altivec_preds, bdesc_abs

I think you lost the last line(s) here?

So, you do not want to create the builtins that we expand to machine insns
that are not supported with the -mcpu= (or other flags) in use.  What does
the ABI have to say about this?


Segher

[PATCH] PR78056: Fix build failure on Power7

[Kelvin Nilsen]

This patch corrects an error introduced with commit 241314.  That patch
introduced several new built-in functions to support Power9 string
instructions.  The error that was found subsequent to the trunk commit
is that initialization of the built-in function tables encounters an
internal compiler error if the assembler that is used with gcc lacks
support for Power9 instructions.

This patch disables initialization of built-in functions which depend
on assembler capabilities that are not supported by the associated tool
chain.

This patch has been booted and regression tested on
powerpcle-unknown-linux, trunk revision 241406.  (I was not able to
regression test on the most current trunk because that trunk does not
boot.)  I have also successfully boot-strapped this patch on a Power7
system for which the assembler lacks support for Power9.  (I could not
regression test on that platform because that platform could not
bootstrap without this patch.)

It is planned that a subsequent enhancement to this patch will make the
following improvements:

1. Fail with an assertion error instead of an internal compiler error
if built-in functions are ever defined for which the corresponding
instruction pattern is not supported by the current compiler
configuration.

2. Issue a warning message whenever a command-line -mcpu=XXX request
seeks to configure support for a CPU version which is not supported by
the accompanying assembler.

I am submitting the patch as is in order to expedite integration since
the error has broken the trunk for certain system configurations.

Is this patch ok for trunk?

gcc/ChangeLog:

2016-10-25  Kelvin Nilsen  

PR target/78056
* config/rs6000/rs6000.c (spe_init_builtins): Modify loops to not
define builtin functions from the bdesc_spe_predicates or
bdesc_spe_evsel arrays if the builtin mask is not compatible with
the current compiler configuration.
(paired_init_builtins): Modify loop to not define define builtin
functions from the bdesc_paried_preds array if the builtin mask is
not compatible with the current compiler configuration.
(altivec_init_builtins): Modify loops to not define the
__builtin_altivec_stxvl function nor the builtin functions from
the bdesc_dst or bdesc_altivec_preds, bdesc_abs

gcc/testsuite/ChangeLog:

2016-10-25  Kelvin Nilsen  

PR target/78056
* gcc.target/powerpc/vsu/vec-any-eqz-7.c (test_any_equal): Change
expected error message.
* gcc.target/powerpc/vsu/vec-xst-len-12.c (store_data): Change
expected error message.
* gcc.target/powerpc/vsu/vec-all-nez-7.c
(test_all_not_equal_and_not_zero): Change expected error message.

Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c  (revision 241406)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -16923,6 +16923,7 @@ spe_init_builtins (void)
   tree pushort_type_node = build_pointer_type (short_unsigned_type_node);
   const struct builtin_description *d;
   size_t i;
+  HOST_WIDE_INT builtin_mask = rs6000_builtin_mask;
 
   tree v2si_ftype_4_v2si
 = build_function_type_list (opaque_V2SI_type_node,
@@ -17063,7 +17064,16 @@ spe_init_builtins (void)
   for (i = 0; i < ARRAY_SIZE (bdesc_spe_predicates); ++i, d++)
 {
   tree type;
+  HOST_WIDE_INT mask = d->mask;
 
+  if ((mask & builtin_mask) != mask)
+   {
+ if (TARGET_DEBUG_BUILTIN)
+   fprintf (stderr, "spe_init_builtins, skip predicate %s\n",
+d->name);
+ continue;
+   }
+
   switch (insn_data[d->icode].operand[1].mode)
{
case V2SImode:
@@ -17084,7 +17094,16 @@ spe_init_builtins (void)
   for (i = 0; i < ARRAY_SIZE (bdesc_spe_evsel); ++i, d++)
 {
   tree type;
+  HOST_WIDE_INT mask = d->mask;
 
+  if ((mask & builtin_mask) != mask)
+   {
+ if (TARGET_DEBUG_BUILTIN)
+   fprintf (stderr, "spe_init_builtins, skip evsel %s\n",
+d->name);
+ continue;
+   }
+
   switch (insn_data[d->icode].operand[1].mode)
{
case V2SImode:
@@ -17106,6 +17125,7 @@ paired_init_builtins (void)
 {
   const struct builtin_description *d;
   size_t i;
+  HOST_WIDE_INT builtin_mask = rs6000_builtin_mask;
 
tree int_ftype_int_v2sf_v2sf
 = build_function_type_list (integer_type_node,
@@ -17141,7 +17161,16 @@ paired_init_builtins (void)
   for (i = 0; i < ARRAY_SIZE (bdesc_paired_preds); ++i, d++)
 {
   tree type;
+  HOST_WIDE_INT mask = d->mask;
 
+  if ((mask & builtin_mask) != mask)
+   {
+ if (TARGET_DEBUG_BUILTIN)
+   fprintf (stderr, "paired_init_builtins, skip predicate %s\n",
+d->name);
+ continue;
+   }
+
   if (TARGET_DEBUG_BUILTIN)
fprintf (stderr, "paired pred #%d, insn