I'm picking up Will's patches for this bug. As an FYI, this is the bug where
_ARCH_PWR8 is conditional on TARGET_DIRECT_MOVE which can be disabled with
-mno-vsx which is bad.
I already posted the cleanup patch that the updated patch for this bug will rely
on, that removed the OPTION_MASK_DIRECT_MOVE because it is fully redundant with
OPTION_MASK_P8_VECTOR. I've also incorporated some of Ke Wen's review comments
on Will's original patch. I have a couple of comments on your review though...
On 10/17/22 1:08 PM, Segher Boessenkool wrote:
> On Mon, Sep 19, 2022 at 11:13:20AM -0500, will schmidt wrote:
>> @@ -24046,10 +24045,11 @@ static struct rs6000_opt_mask const
>> rs6000_opt_masks[] =
>> { "block-ops-vector-pair", OPTION_MASK_BLOCK_OPS_VECTOR_PAIR,
>> false, true },
>> { "cmpb", OPTION_MASK_CMPB, false, true },
>> { "crypto", OPTION_MASK_CRYPTO, false,
>> true },
>> { "direct-move", OPTION_MASK_DIRECT_MOVE, false, true },
>> + { "power8", OPTION_MASK_POWER8, false,
>> true },
>
> Why would we want a #pragma power8 ?
Agreed, we don't want that. We have target attribute cpu=power8 for that.
>> +mpower8
>> +Target Mask(POWER8) Var(rs6000_isa_flags)
>> +Use instructions added in ISA 2.07 (power8).
>
> There should not be such an option. It is set by -mcpu=power8 and
> later, but can never be enabled or disabled direfctly by the user.
So we need an OPTION_MASK_POWER8 to be created for use in rs6000_isa_flags, but
the only way I see that we can do that is to create an option in rs6000.opt.
Did I miss that there is another way? Otherwise, I was thinking of creating a
dummy option that is WarnRemoved from the start ala:
+;; This option exists only for its MASK. It is not intended for users.
+mpower8
+Target Mask(POWER8) Var(rs6000_isa_flags) WarnRemoved
+
Is there a better way? The problem is P8 created lots of new instructions, but
they were basically all vector and htm instructions. There were no general
GPR or FPR instructions (ie, what we'd think of as base architecture) added,
so there's no other OPTION_MASK_*/TARGET_* we can use as a P8 base architecture
test.
I'll note I tried just a bare "Target Mask(POWER8) Var(rs6000_isa_flags)" with
no
option name mentioned at all, but that didn't work, as no OPTION_MASK_POWER8 was
created.
Peter
