On 8/24/23 12:56 AM, Kewen.Lin wrote:
> By looking into the uses of function rs6000_pcrel_p, I think we can
> just replace it with TARGET_PCREL. Previously we don't require PCREL
> unset for any unsupported target/OS, so we need rs6000_pcrel_p() to
> ensure it's really supported in those use places, now if we can guarantee
> TARGET_PCREL only hold for all places where it's supported, it looks
> we can just check TARGET_PCREL only?
I think you're correct on only needing TARGET_PCREL instead of rs6000_pcrel_p()
as long as we correctly disable PCREL on the targets that either don't support
it or those that due, but don't due to explicit options (ie, -mno-prel or
ELFv2 and -mcmodel != medium, etc.).
> Then the code structure can look like:
>
> if (PCREL_SUPPORTED_BY_OS
> && (rs6000_isa_flags_explicit & OPTION_MASK_PCREL) == 0)
> // enable
> else if (TARGET_PCREL && DEFAULT_ABI != ABI_ELFv2)
> // disable
> else if (TARGET_PCREL && TARGET_CMODEL != CMODEL_MEDIUM)
> // disable
I don't like that first conditional expression. The problem is,
PCREL_SUPPORTED_BY_OS could be true or false for the following
tests, because it's anded with the explicit option test, and
sometimes that won't make sense. I think something safer is
something like:
if (PCREL_SUPPORTED_BY_OS)
{
/* PCREL on ELFv2 requires -mcmodel=medium. */
if (DEFAULT_ABI == ABI_ELFv2 && TARGET_CMODEL != CMODEL_MEDIUM)
error ("%qs requires %qs", "-mpcrel", "-mcmodel=medium");
if ((rs6000_isa_flags_explicit & OPTION_MASK_PCREL) == 0)
rs6000_isa_flags |= OPTION_MASK_PCREL;
}
else
{
if ((rs6000_isa_flags_explicit & OPTION_MASK_PCREL) != 0
&& TARGET_PCREL)
error ("use of %qs is invalid for this target", "-mpcrel");
rs6000_isa_flags &= ~OPTION_MASK_PCREL;
}
...although, even the cmodel != medium test above should probably have
some extra tests to ensure TARGET_PCREL is true (and explicit?) and
mcmodel != medium before giving an error??? Ie, a ELFv2 P10 compile
with an implicit -mpcrel and explicit -mcmodel={small,large} probably
should not error and just silently disable pcrel??? Meaning only
when we explicitly say -mpcrel -mcmodel={small,large} should we give
that error. Thoughts on that?
Peter
