Hi Carl,

On Wed, Mar 18, 2020 at 04:19:18PM -0700, Carl Love wrote:
> > Yes, but only for this fprnd vs. 2.06 (vsx) situation.  Like we
> > already
> > have:
> > 
> >   if (TARGET_DIRECT_MOVE && !TARGET_VSX)
> >     {
> >       if (rs6000_isa_flags_explicit & OPTION_MASK_DIRECT_MOVE)
> >         error ("%qs requires %qs", "-mdirect-move", "-mvsx");
> >       rs6000_isa_flags &= ~OPTION_MASK_DIRECT_MOVE;
> >     }
> > 
> > (and many other cases there), we could do this there as well (so,
> > don't
> > allow -mvsx (maybe via a -mcpu= etc.) at the same time as -mno-
> > fprnd).
> 
> I redid the patch to try and make it more general.  It looks to me like
> TARGET_VSX is set for Power 7 and newer.

By default, yes.  But you can use -mno-vsx, and you can use -mvsx with
older CPUs as well (but you really really really shouldn't).

> I setup a test similar to the
> example checking TARGET_VSX. So if you are on a Power 7 then -mvsx is
> set for you, i.e. the user would not have to explicitly use the option.
> My objection to the error message in the example is that the user
> wouldn't necessarily know what processor or ISA is implied by -mvsx. 
> So in my error message I called out the processor number.  We could do
> it based on ISA.  I figure the user is more likely to know the
> processor version then the ISA level supported by the processor so went
> with the processor number in the patch.  Thoughts?
> 
> gcc -mno-fprnd -g -mcpu=power7 -c vsx-builtin-3.c
> cc1: error: ‘-mno-fprnd’ not compatible with Power 7 and newer

I think it should say

        error ("%qs requires %qs", "-mvsx", "-mfprnd");

(It's easier to not use negatives, and, this is more consistent with
other such tests).

>       * gcc/config/rs6000/rs6000.c (altivec_resolve_overloaded_builtin):
>       Add check for TARGET_FRND for Power 7 or newer.

(It's in a different function now, and, TARGET_FPRND).

> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index ac9455e3b7c..5c72a863dbf 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -3716,6 +3716,14 @@ rs6000_option_override_internal (bool global_init_p)
>        rs6000_isa_flags &= ~OPTION_MASK_CRYPTO;
>      }
>  
> +  if (!TARGET_FPRND && TARGET_VSX)
> +    {
> +      if (rs6000_isa_flags_explicit & OPTION_MASK_FPRND)
> +     /* TARGET_VSX = 1 implies Power 7 and newer */
> +     error ("%qs not compatible with Power 7 and newer", "-mno-fprnd");
> +      rs6000_isa_flags &= ~OPTION_MASK_FPRND;
> +    }

Please make such changes if you agree.  Either way, okay for trunk.
Thank you, and sorry the review took so long.


Segher

Reply via email to