[rs6000] Avoid useless masking of count operand for rotation
Eric Botcazou
botcazou@adacore.com
Fri Oct 2 08:26:24 GMT 2020
> Don't call it "mask" please: *all* of our basic rotate instructions
> already have something called "mask" (that is the "m" in "rlwnm" for
> example; and "rotlw d,a,b" is just an extended mnemonic for
> "rlwnm d,a,b,0,31"). The hardware also does not mask the shift amount
> at all (instead, only the low 5 bits of RB *are* the shift amount).
"Mask" is a common terminology though, used e.g. in the x86 back-end. That
being said, I agree that it conflicts with PowerPC specific stuff so please
suggest an alternative here.
> > +(define_insn "*rotl<mode>3_mask"
> > + [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
> > + (rotate:GPR (match_operand:GPR 1 "gpc_reg_operand" "r")
> > + (and:GPR (match_operand:GPR 2 "gpc_reg_operand" "r")
> > + (match_operand:GPR 3 "const_int_operand"
"n"))))]
> > + "(UINTVAL (operands[3]) & (GET_MODE_BITSIZE (<MODE>mode) - 1))
> > + == (unsigned HOST_WIDE_INT) (GET_MODE_BITSIZE (<MODE>mode) - 1)"
>
> (Useless casts are useless.)
OK, I can remove it.
> Don't mask operands[3] please (in the UINTVAL): RTL with the number
> outside of that range is *undefined*. So just check that it is equal?
Copied from the x86 back-end as well, so I'd rather keep it that way, unless
you really insist...
> Why? This diverges the "dot" version from the non-dot for no reason.
OK, let's drop it.
> I don't see a patch with splitters only? Huh. Did you attach the same
> patch twice?
AFAICS no, 2 different patches were attached.
> Since this won't be handled before combine (or what do I miss?), it is
> fine to do splitters only (splitters for combine). But the other
> approach is fine as well.
Patch #2 uses define_and_split like the x86 back-end; moreover, we already
have define_and_split for the dot variants so maybe it's the best way to go?
--
Eric Botcazou
More information about the Gcc-patches
mailing list