Hi!

On Wed, Mar 11, 2020 at 12:00:12PM -0700, Carl Love wrote:
> The following patch add a check to make sure the user did not specify 
> -mno_fprnd with the builtins  __builtin_vsx_xsrdpim and
> __builtin_vsx_xsrdpip.  These builtins are incompatible with the
> -mno_fprnd command line.  The check prevents GCC crashing under these
> conditions.

(-mno-fprnd, a dash, not an underscore)

-mfprnd means all new insns in ISA 2.04; we should never allow this
option together with a -mcpu= that implies 2.04 or higher.

xsrdpi* require ISA 2.06 (Power7), so this testcase should work *anyway*,
even if fri* would be disabled for some strange reason.

> I read thru the source code looking for other builtins with the same
> issue.

>From the GCC manual:

  -mmfcrf    p4  2.01
  -mpopcntb  p5  2.02
  -mfprnd    p5+ 2.04  ("info gcc" says 2.03, that's wrong?  But the ISA
                        says this is 2.02 even?  Now what!)
  -mcmpb     p6  2.05
  -mpopcntd  p7  2.06

(and there are more, in fact).

> 2020-03-10  Carl Love  <c...@us.ibm.com>
> 
>       * gcc/config/rs6000/rs6000-c.c
> (altivec_resolve_overloaded_builtin):
>       Add check for TARGET_FRND and VSX_BUILTIN_XSRDPIM,
> VSX_BUILTIN_XSRDPIP
>       compatibility.

FPRND

> --- a/gcc/config/rs6000/rs6000-c.c
> +++ b/gcc/config/rs6000/rs6000-c.c
> @@ -915,6 +915,14 @@ altivec_resolve_overloaded_builtin (location_t
> loc, tree fndecl,
>    const struct altivec_builtin_types *desc;
>    unsigned int n;
>  
> +  /* Check builtin for command line argument conflicts.  */
> +  if (!TARGET_FPRND &&
> +      (fcode == VSX_BUILTIN_XSRDPIM || fcode == VSX_BUILTIN_XSRDPIP))

Lines never end in binary operators: that goes to the start of the next
line, instead, so

  if (!TARGET_FPRND
      && (fcode == VSX_BUILTIN_XSRDPIM || fcode == VSX_BUILTIN_XSRDPIP))

I'd write that the other way around, it reads nicer:

  if ((fcode == VSX_BUILTIN_XSRDPIM || fcode == VSX_BUILTIN_XSRDPIP)
      && !TARGET_FPRND)

but maybe that is just taste :-)

> {
> +      error ("%s is incompatible with mno-fprnd option",
> +          IDENTIFIER_POINTER (DECL_NAME (fndecl)));
> +      return error_mark_node;
> +  }

-mno-fprnd (options start with a dash, except in the first field in
rs6000.opt).

It would be better if you disallowed this option combination?  Don't
allow an options that says ISA X insns are allowed, but ISA Y insns are
not, with Y < X.  In this case X is 2.06 and Y is 2.02 or 2.03 or 2.04.


Segher

Reply via email to