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