On Wed, May 29, 2019 at 03:26:27PM -0500, Bill Schmidt wrote:
> On 5/29/19 8:16 AM, Segher Boessenkool wrote:
> > Hi!
> >
> > On Wed, May 29, 2019 at 07:42:38AM -0500, Bill Schmidt wrote:
> >> * rs6000-cpus.def (OTHER_FUSION_MASKS): New #define.
> >> (ISA_FUTURE_MASKS_SERVER): Add OPTION_MASK_PREFIXED_ADDR. Mask off
> >> OTHER_FUSION_MASKS.
> > Two spaces after a full stop (here and later again).
>
> Oops, yep.
> >
> >> +/* ISA masks setting fusion options. */
> >> +#define OTHER_FUSION_MASKS (OPTION_MASK_P8_FUSION
> >> \
> >> + | OPTION_MASK_P8_FUSION_SIGN)
> > Or merge the two masks into one?
>
> I'll ask Mike to explain this, as I don't know why there are two masks.
The intention is to allow for using the numeric prefixed instructions without
pc-relative. I.e.
long c = 0x12345;
would generate:
PLI r,0x12345
Instead of ADDI/ADDIS, but still not generate the pc-relative instructions.
The intention was when we get to timing stuff, there are fewer differences. I
could live with dropping prefixed-addr as a separate switch.
However, since there are other prefixed instructions that we will do someday
that aren't necessarily using pc-relative, if we drop it, we need to make sure
all of the prefixed stuff is now targetted on -mfuture instead of
-mprefixed-addr.
> >
> >> /* Support for a future processor's features. */
> >> -#define ISA_FUTURE_MASKS_SERVER (ISA_3_0_MASKS_SERVER
> >> \
> >> - | OPTION_MASK_FUTURE \
> >> - | OPTION_MASK_PCREL)
> >> +#define ISA_FUTURE_MASKS_SERVER ((ISA_3_0_MASKS_SERVER
> >> \
> >> + | OPTION_MASK_FUTURE \
> >> + | OPTION_MASK_PCREL \
> >> + | OPTION_MASK_PREFIXED_ADDR) \
> >> + & ~OTHER_FUSION_MASKS)
> > OTHER_FUSION_MASKS shouldn't be part of ISA_3_0_MASKS_SERVER. Fix that
> > instead? Fusion is a property of specific CPUs, not of ISA versions.
>
> Agreed, I think this should be masked out of ISA_3_0_MASKS_SERVER, but
> would like Mike to agree before I change it in case I'm missing
> something obvious.
Mostly it was me being cautious that I wasn't sure all of the p8 fusion stuff
would play well with prefixed addressing (since the p8 fusion stuff does use
peephole2 and it might not be prepared for prefixed instructions).
> >
> >> - /* -mpcrel requires the prefixed load/store support on FUTURE systems.
> >> */
> >> - if (!TARGET_FUTURE && TARGET_PCREL)
> >> + /* -mprefixed-addr and -mpcrel require the prefixed load/store support
> >> on
> >> + FUTURE systems. */
> >> + if (!TARGET_FUTURE && (TARGET_PCREL || TARGET_PREFIXED_ADDR))
> >> {
> >> if ((rs6000_isa_flags_explicit & OPTION_MASK_PCREL) != 0)
> >> error ("%qs requires %qs", "-mpcrel", "-mcpu=future");
> > PCREL requires PREFIXED_ADDR, please simplify.
> >
> >> + if (TARGET_PCREL && !TARGET_PREFIXED_ADDR)
> >> + {
> >> + if ((rs6000_isa_flags_explicit & OPTION_MASK_PCREL) != 0)
> >> + error ("%qs requires %qs", "-mpcrel", "-mprefixed-addr");
> >> +
> >> rs6000_isa_flags &= ~OPTION_MASK_PCREL;
> >> }
> > Maybe put this test first, if that makes things easier or more logical?
> ok
Yes, I was working on this with my no-pcrel default patch I was about to submit.
> >
> >> @@ -36379,6 +36391,7 @@ static struct rs6000_opt_mask const
> >> rs6000_opt_masks[] =
> >> { "power9-vector", OPTION_MASK_P9_VECTOR, false,
> >> true },
> >> { "powerpc-gfxopt", OPTION_MASK_PPC_GFXOPT, false,
> >> true },
> >> { "powerpc-gpopt", OPTION_MASK_PPC_GPOPT, false,
> >> true },
> >> + { "prefixed-addr", OPTION_MASK_PREFIXED_ADDR, false,
> >> true },
> > Do we want this? Why?
>
> Performance folks are using it for testing purposes. Eventually this
> will probably drop out, but for now I think it's best to have the
> undocumented switch.
I use that table with -mdebug=reg so I can make sure exactly what options are
on or off. Please add any undocumented switch to the table.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: [email protected], phone: +1 (978) 899-4797
