* [PATCH] rs6000: Add undocumented switch -mprefixed-addr
@ 2019-05-29 12:53 Bill Schmidt
2019-05-29 13:21 ` Segher Boessenkool
0 siblings, 1 reply; 8+ messages in thread
From: Bill Schmidt @ 2019-05-29 12:53 UTC (permalink / raw)
To: GCC Patches; +Cc: Segher Boessenkool
Hi,
This patch adds the undocumented switch -mprefixed-addr and a little of the basic
infrastructure around it. It also includes a couple of lines in the same code
regions to disable P8 fusion for -mcpu=future.
Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no regressions.
Is this okay for trunk?
Thanks,
Bill
2019-05-28 Bill Schmidt <wschmidt@linux.ibm.com>
Michael Meissner <meissner@linux.ibm.com>
* rs6000-cpus.def (OTHER_FUSION_MASKS): New #define.
(ISA_FUTURE_MASKS_SERVER): Add OPTION_MASK_PREFIXED_ADDR. Mask off
OTHER_FUSION_MASKS.
(OTHER_FUTURE_MASKS): Add OPTION_MASK_PREFIXED_ADDR.
(POWERPC_MASKS): Likewise.
* rs6000.c (rs6000_option_override_internal): Error if
-mprefixed-addr is specified without -mcpu=future. Error if
-mpcrel is specified without -mprefixed-addr.
(rs6000_opt_masks): Add entry for prefixed-addr.
* rs6000.opt (mprefixed-addr): New option.
diff --git a/gcc/config/rs6000/rs6000-cpus.def b/gcc/config/rs6000/rs6000-cpus.def
index 5337382bdcf..2fb612a8401 100644
--- a/gcc/config/rs6000/rs6000-cpus.def
+++ b/gcc/config/rs6000/rs6000-cpus.def
@@ -72,13 +72,20 @@
| OPTION_MASK_P9_VECTOR \
| OPTION_MASK_DIRECT_MOVE)
+/* ISA masks setting fusion options. */
+#define OTHER_FUSION_MASKS (OPTION_MASK_P8_FUSION \
+ | OPTION_MASK_P8_FUSION_SIGN)
+
/* 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)
/* Flags that need to be turned off if -mno-future. */
-#define OTHER_FUTURE_MASKS (OPTION_MASK_PCREL)
+#define OTHER_FUTURE_MASKS (OPTION_MASK_PCREL \
+ | OPTION_MASK_PREFIXED_ADDR)
/* Flags that need to be turned off if -mno-power9-vector. */
#define OTHER_P9_VECTOR_MASKS (OPTION_MASK_FLOAT128_HW \
@@ -139,6 +146,7 @@
| OPTION_MASK_POWERPC64 \
| OPTION_MASK_PPC_GFXOPT \
| OPTION_MASK_PPC_GPOPT \
+ | OPTION_MASK_PREFIXED_ADDR \
| OPTION_MASK_QUAD_MEMORY \
| OPTION_MASK_QUAD_MEMORY_ATOMIC \
| OPTION_MASK_RECIP_PRECISION \
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 9229bad6acc..1860b344c3a 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4296,12 +4296,24 @@ rs6000_option_override_internal (bool global_init_p)
rs6000_isa_flags &= ~OPTION_MASK_FLOAT128_HW;
}
- /* -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");
+ else if ((rs6000_isa_flags_explicit & OPTION_MASK_PREFIXED_ADDR) != 0)
+ error ("%qs requires %qs", "-mprefixed-addr", "-mcpu=future");
+
+ rs6000_isa_flags &= ~(OPTION_MASK_PCREL | OPTION_MASK_PREFIXED_ADDR);
+ }
+
+ 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;
}
@@ -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 },
{ "quad-memory", OPTION_MASK_QUAD_MEMORY, false, true },
{ "quad-memory-atomic", OPTION_MASK_QUAD_MEMORY_ATOMIC, false, true },
{ "recip-precision", OPTION_MASK_RECIP_PRECISION, false, true },
diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
index 43b04834746..3a4353674b8 100644
--- a/gcc/config/rs6000/rs6000.opt
+++ b/gcc/config/rs6000/rs6000.opt
@@ -574,6 +574,10 @@ mfuture
Target Report Mask(FUTURE) Var(rs6000_isa_flags)
Use instructions for a future architecture.
+mprefixed-addr
+Target Undocumented Mask(PREFIXED_ADDR) Var(rs6000_isa_flags)
+Generate (do not generate) prefixed memory instructions.
+
mpcrel
Target Report Mask(PCREL) Var(rs6000_isa_flags)
Generate (do not generate) pc-relative memory addressing.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rs6000: Add undocumented switch -mprefixed-addr
2019-05-29 12:53 [PATCH] rs6000: Add undocumented switch -mprefixed-addr Bill Schmidt
@ 2019-05-29 13:21 ` Segher Boessenkool
2019-05-29 21:25 ` Bill Schmidt
0 siblings, 1 reply; 8+ messages in thread
From: Segher Boessenkool @ 2019-05-29 13:21 UTC (permalink / raw)
To: Bill Schmidt; +Cc: GCC Patches
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).
> +/* 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?
> /* 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.
> - /* -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?
> @@ -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?
Segher
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rs6000: Add undocumented switch -mprefixed-addr
2019-05-29 13:21 ` Segher Boessenkool
@ 2019-05-29 21:25 ` Bill Schmidt
2019-05-29 22:00 ` Segher Boessenkool
2019-05-30 16:16 ` [PATCH] " Michael Meissner
0 siblings, 2 replies; 8+ messages in thread
From: Bill Schmidt @ 2019-05-29 21:25 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: GCC Patches, Michael Meissner
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.
>
>> /* 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.
>
>> - /* -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
>
>> @@ -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.
Thanks,
Bill
>
>
> Segher
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rs6000: Add undocumented switch -mprefixed-addr
2019-05-29 21:25 ` Bill Schmidt
@ 2019-05-29 22:00 ` Segher Boessenkool
2019-05-30 0:57 ` [PATCH v2] " Bill Schmidt
2019-05-30 16:16 ` [PATCH] " Michael Meissner
1 sibling, 1 reply; 8+ messages in thread
From: Segher Boessenkool @ 2019-05-29 22:00 UTC (permalink / raw)
To: Bill Schmidt; +Cc: GCC Patches, Michael Meissner
On Wed, May 29, 2019 at 03:26:27PM -0500, Bill Schmidt wrote:
> >> @@ -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.
Command-line options, sure, but is that what this code is about? Huh.
I thought it was for attribute target and that stuff.
Oh well, okay either way. For now :-)
Segher
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] rs6000: Add undocumented switch -mprefixed-addr
2019-05-29 22:00 ` Segher Boessenkool
@ 2019-05-30 0:57 ` Bill Schmidt
2019-05-30 15:14 ` Segher Boessenkool
0 siblings, 1 reply; 8+ messages in thread
From: Bill Schmidt @ 2019-05-30 0:57 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: GCC Patches, Michael Meissner
Hi,
Here's another version of the -mprefixed-addr patch. I've attempted to address
Segher's comments, other than the suggestion to merge OPTION_MASK_P8_FUSION_SIGN
into OPTION_MASK_P8_FUSION, which I think is too big a rat's nest for this patch
series.
Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no regressions.
Is this version okay for trunk?
Thanks!
Bill
2019-05-29 Bill Schmidt <wschmidt@linux.ibm.com>
Michael Meissner <meissner@linux.ibm.com>
* rs6000-cpus.def (OTHER_FUSION_MASKS): New #define.
(ISA_3_0_MASKS_SERVER): Mask off OTHER_FUSION_MASKS.
(ISA_3_0_MASKS_IEEE): Remove OPTION_MASK_DIRECT_MOVE.
(ISA_FUTURE_MASKS_SERVER): Add OPTION_MASK_PREFIXED_ADDR.
(OTHER_FUTURE_MASKS): Likewise.
(POWERPC_MASKS): Likewise.
* rs6000.c (rs6000_option_override_internal): Error if -mpcrel is
specified without -mprefixed-addr or -mcpu=future. Error if
-mprefied-addr is specified without -mcpu=future.
(rs6000_opt_masks): Add entry for prefixed-addr.
* rs6000.opt (mprefixed-addr): New option.
diff --git a/gcc/config/rs6000/rs6000-cpus.def b/gcc/config/rs6000/rs6000-cpus.def
index 5337382bdcf..101efd54183 100644
--- a/gcc/config/rs6000/rs6000-cpus.def
+++ b/gcc/config/rs6000/rs6000-cpus.def
@@ -56,29 +56,35 @@
| OPTION_MASK_QUAD_MEMORY \
| OPTION_MASK_QUAD_MEMORY_ATOMIC)
+/* ISA masks setting fusion options. */
+#define OTHER_FUSION_MASKS (OPTION_MASK_P8_FUSION \
+ | OPTION_MASK_P8_FUSION_SIGN)
+
/* Add ISEL back into ISA 3.0, since it is supposed to be a win. Do not add
FLOAT128_HW here until we are ready to make -mfloat128 on by default. */
-#define ISA_3_0_MASKS_SERVER (ISA_2_7_MASKS_SERVER \
- | OPTION_MASK_ISEL \
- | OPTION_MASK_MODULO \
- | OPTION_MASK_P9_MINMAX \
- | OPTION_MASK_P9_MISC \
- | OPTION_MASK_P9_VECTOR)
+#define ISA_3_0_MASKS_SERVER ((ISA_2_7_MASKS_SERVER \
+ | OPTION_MASK_ISEL \
+ | OPTION_MASK_MODULO \
+ | OPTION_MASK_P9_MINMAX \
+ | OPTION_MASK_P9_MISC \
+ | OPTION_MASK_P9_VECTOR) \
+ & ~OTHER_FUSION_MASKS)
/* Support for the IEEE 128-bit floating point hardware requires a lot of the
VSX instructions that are part of ISA 3.0. */
#define ISA_3_0_MASKS_IEEE (OPTION_MASK_VSX \
| OPTION_MASK_P8_VECTOR \
- | OPTION_MASK_P9_VECTOR \
- | OPTION_MASK_DIRECT_MOVE)
+ | OPTION_MASK_P9_VECTOR)
/* Support for a future processor's features. */
#define ISA_FUTURE_MASKS_SERVER (ISA_3_0_MASKS_SERVER \
| OPTION_MASK_FUTURE \
- | OPTION_MASK_PCREL)
+ | OPTION_MASK_PCREL \
+ | OPTION_MASK_PREFIXED_ADDR)
/* Flags that need to be turned off if -mno-future. */
-#define OTHER_FUTURE_MASKS (OPTION_MASK_PCREL)
+#define OTHER_FUTURE_MASKS (OPTION_MASK_PCREL \
+ | OPTION_MASK_PREFIXED_ADDR)
/* Flags that need to be turned off if -mno-power9-vector. */
#define OTHER_P9_VECTOR_MASKS (OPTION_MASK_FLOAT128_HW \
@@ -139,6 +145,7 @@
| OPTION_MASK_POWERPC64 \
| OPTION_MASK_PPC_GFXOPT \
| OPTION_MASK_PPC_GPOPT \
+ | OPTION_MASK_PREFIXED_ADDR \
| OPTION_MASK_QUAD_MEMORY \
| OPTION_MASK_QUAD_MEMORY_ATOMIC \
| OPTION_MASK_RECIP_PRECISION \
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index b9fd89837a6..f9ef8e38314 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4296,15 +4296,24 @@ rs6000_option_override_internal (bool global_init_p)
rs6000_isa_flags &= ~OPTION_MASK_FLOAT128_HW;
}
- /* -mpcrel requires the prefixed load/store support on FUTURE systems. */
- if (!TARGET_FUTURE && TARGET_PCREL)
+ /* -mpcrel requires prefixed load/store addressing. */
+ if (TARGET_PCREL && !TARGET_PREFIXED_ADDR)
{
if ((rs6000_isa_flags_explicit & OPTION_MASK_PCREL) != 0)
- error ("%qs requires %qs", "-mpcrel", "-mcpu=future");
+ error ("%qs requires %qs", "-mpcrel", "-mprefixed-addr");
rs6000_isa_flags &= ~OPTION_MASK_PCREL;
}
+ /* -mprefixed-addr (and hence -mpcrel) requires -mcpu=future. */
+ if (TARGET_PREFIXED_ADDR && !TARGET_FUTURE)
+ {
+ if ((rs6000_isa_flags_explicit & OPTION_MASK_PCREL) != 0)
+ error ("%qs requires %qs", "-mprefixed-addr", "-mcpu=future");
+
+ rs6000_isa_flags &= ~(OPTION_MASK_PCREL | OPTION_MASK_PREFIXED_ADDR);
+ }
+
/* Print the options after updating the defaults. */
if (TARGET_DEBUG_REG || TARGET_DEBUG_TARGET)
rs6000_print_isa_options (stderr, 0, "after defaults", rs6000_isa_flags);
@@ -36376,6 +36385,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 },
{ "quad-memory", OPTION_MASK_QUAD_MEMORY, false, true },
{ "quad-memory-atomic", OPTION_MASK_QUAD_MEMORY_ATOMIC, false, true },
{ "recip-precision", OPTION_MASK_RECIP_PRECISION, false, true },
diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
index 43b04834746..3a4353674b8 100644
--- a/gcc/config/rs6000/rs6000.opt
+++ b/gcc/config/rs6000/rs6000.opt
@@ -574,6 +574,10 @@ mfuture
Target Report Mask(FUTURE) Var(rs6000_isa_flags)
Use instructions for a future architecture.
+mprefixed-addr
+Target Undocumented Mask(PREFIXED_ADDR) Var(rs6000_isa_flags)
+Generate (do not generate) prefixed memory instructions.
+
mpcrel
Target Report Mask(PCREL) Var(rs6000_isa_flags)
Generate (do not generate) pc-relative memory addressing.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] rs6000: Add undocumented switch -mprefixed-addr
2019-05-30 0:57 ` [PATCH v2] " Bill Schmidt
@ 2019-05-30 15:14 ` Segher Boessenkool
0 siblings, 0 replies; 8+ messages in thread
From: Segher Boessenkool @ 2019-05-30 15:14 UTC (permalink / raw)
To: Bill Schmidt; +Cc: GCC Patches, Michael Meissner
On Wed, May 29, 2019 at 07:09:47PM -0500, Bill Schmidt wrote:
> Here's another version of the -mprefixed-addr patch. I've attempted to address
> Segher's comments, other than the suggestion to merge OPTION_MASK_P8_FUSION_SIGN
> into OPTION_MASK_P8_FUSION, which I think is too big a rat's nest for this patch
> series.
Yeah, probably a wise idea. But we'll need to untangle it some day :-(
> * rs6000.c (rs6000_option_override_internal): Error if -mpcrel is
> specified without -mprefixed-addr or -mcpu=future. Error if
> -mprefied-addr is specified without -mcpu=future.
Typo. (Aspell thinks this should be "purified", heh. It also suggests
"putrefied". But not the obvious "prefried").
Okay for trunk with that fixed. Thanks!
Segher
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rs6000: Add undocumented switch -mprefixed-addr
2019-05-29 21:25 ` Bill Schmidt
2019-05-29 22:00 ` Segher Boessenkool
@ 2019-05-30 16:16 ` Michael Meissner
2019-06-02 23:01 ` Segher Boessenkool
1 sibling, 1 reply; 8+ messages in thread
From: Michael Meissner @ 2019-05-30 16:16 UTC (permalink / raw)
To: Bill Schmidt; +Cc: Segher Boessenkool, GCC Patches, Michael Meissner
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: meissner@linux.ibm.com, phone: +1 (978) 899-4797
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rs6000: Add undocumented switch -mprefixed-addr
2019-05-30 16:16 ` [PATCH] " Michael Meissner
@ 2019-06-02 23:01 ` Segher Boessenkool
0 siblings, 0 replies; 8+ messages in thread
From: Segher Boessenkool @ 2019-06-02 23:01 UTC (permalink / raw)
To: Michael Meissner, Bill Schmidt, GCC Patches
On Thu, May 30, 2019 at 12:01:45PM -0400, Michael Meissner wrote:
> On Wed, May 29, 2019 at 03:26:27PM -0500, Bill Schmidt wrote:
> > On 5/29/19 8:16 AM, Segher Boessenkool wrote:
> > >> +/* 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.
[ snip ]
I was suggesting merging these two P8_FUSION{,_SIGN} into one. But, we'll
get to that some day, it doesn't have to be now.
> > >> @@ -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.
It's not very nice to have to edit everything in two completely separate
places like this.
Segher
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-06-02 23:01 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-29 12:53 [PATCH] rs6000: Add undocumented switch -mprefixed-addr Bill Schmidt
2019-05-29 13:21 ` Segher Boessenkool
2019-05-29 21:25 ` Bill Schmidt
2019-05-29 22:00 ` Segher Boessenkool
2019-05-30 0:57 ` [PATCH v2] " Bill Schmidt
2019-05-30 15:14 ` Segher Boessenkool
2019-05-30 16:16 ` [PATCH] " Michael Meissner
2019-06-02 23:01 ` Segher Boessenkool
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).