On 8/9/24 12:54 PM, Segher Boessenkool wrote: >> --- a/gcc/config/rs6000/altivec.md >> +++ b/gcc/config/rs6000/altivec.md >> @@ -623,7 +623,7 @@ (define_insn "altivec_eqv1ti" >> [(set (match_operand:V1TI 0 "altivec_register_operand" "=v") >> (eq:V1TI (match_operand:V1TI 1 "altivec_register_operand" "v") >> (match_operand:V1TI 2 "altivec_register_operand" "v")))] >> - "TARGET_POWER10" >> + "TARGET_P10_VECTOR" >> "vcmpequq %0,%1,%2" >> [(set_attr "type" "veccmpfx")]) > > This very first one is incorrect, already. This is a Vector insn > (it needs MSR[VEC]=1), not a VSX insn (for which MSR[VSX]=1 is needed). > > We test TARGET_ALTIVEC for that, not TARGET_VSX.
I guess you are correct that *_VECTOR is not specific enough because yeah, we could have -mcpu=power10 -maltivec -mno-vsx so we'd need two macros, TARGET_P10_ALTIVEC and TARGET_P10_VSX rather than one catch-all. That said... > In general, we want to get rid of TARGET_Pxxx_VECTOR, not introduce new > stuff like it! I'm fine with the TARGET_P10_* macro, since it's more readable than saying TARGET_POWER10 && TARGET_ALTIVEC && TARGET_VSX, especially when we use the negated version. What we really wanted to get rid of, was having a separate OPTION_MASK_Pxxx_* flag for things like this which we used to have. I agree that was bad. This isn't that though. This is just a convenience macro to make the code more (IMHO) readable. Peter
