[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