> 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
