Hi!

On Wed, May 22, 2019 at 03:29:17PM -0500, Bill Schmidt wrote:
> +  /* -mpcrel requires the prefixed load/store support on FUTURE systems.  */
> +  if (!TARGET_FUTURE && TARGET_PCREL)
> +    {
> +      if ((rs6000_isa_flags_explicit & OPTION_MASK_PCREL) != 0)
> +     error ("%qs requires %qs", "-mpcrel", "-mcpu=future");
> +
> +      rs6000_isa_flags &= ~OPTION_MASK_PCREL;
> +    }

The comment is confusing...  I read it as "On FUTURE, pcrel requires
prefixed load/store".  The code doesn't even mention prefix, anyway.

Just say "-mpcrel requires -mcpu=future", just like the code says?  Or just
"-mpcrel requires prefix".  Something like that.  "FUTURE is needed for
prefix support"?

> +/* Return whether we should generate PC-relative code for *FN.  */
> +bool
> +rs6000_pcrel_p (struct function *fn)
> +{
> +  if (DEFAULT_ABI != ABI_ELFv2)
> +    return false;
> +
> +  /* Optimize usual case.  */
> +  if (fn == cfun)

Maybe have a separate (helper) function for this?  Defined in the header,
if performance matters.

Looks good.  Okay for trunk.  Thanks!


Segher

Reply via email to