Hi!
On Fri, Jun 17, 2022 at 07:13:37AM +0200, Roger Sayle wrote:
> This patch addresses PR target/105991 where a change to prefer representing
> shifts and adds at the tree-level as multiplications, causes problems for
> the rldimi patterns in the powerpc backend.
Because it now is converted to different RTL at expand time. Which the
generic expand code does some premature optimisation on, which makes us
end up with the addition instead of data manipulation insns. Oh well.
> The issue is that rs6000.md
> models this pattern using IOR, and some variants that have the equivalent
> PLUS or XOR in the RTL fail to match some *rotl<mode>4_insert patterns.
> This is fixed in this patch by adding a define_insn_and_split to locally
> canonicalize the PLUS and XOR forms to the backend's preferred IOR form.
Okay.
> An alternative fix might be for the RTL optimizers to define a canonical
> form for these plus_xor_ior equivalent expressions, but the logical
> choice might be plus (which may appear in an addressing mode), and such
> a change may require a number of tweaks to update various backends
> (i.e. a more intrusive change than the one proposed here).
This does not make sense in an address at all, thankfully :-)
The only sane canonicalisation for this is something like VEC_DUPLICATE
but for submodes of integer modes, instead of the component mode of a
vector mode. I don't feel this is worth trying to handle in general
though.
> Many thanks for Marek Polacek for bootstrapping and regression testing
> this change without problems.
You have an account on the cfarm, it is quick and easy to test there :-)
I recommend gcc135, a 32 core p9, with oodles of disk space :-)
> +; Canonicalize the PLUS and XOR forms to IOR for rotl<mode>3_insert_3
> +(define_code_iterator plus_xor [plus xor])
> +
> +(define_insn_and_split "*rotl<mode>3_insert_3_<code>"
> + [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
> + (plus_xor:GPR
> + (and:GPR (match_operand:GPR 3 "gpc_reg_operand" "0")
> + (match_operand:GPR 4 "const_int_operand" "n"))
> + (ashift:GPR (match_operand:GPR 1 "gpc_reg_operand" "r")
> + (match_operand:SI 2 "const_int_operand" "n"))))]
> + "INTVAL (operands[2]) == exact_log2 (UINTVAL (operands[4]) + 1)"
exact_log2 returns -1 if its argument is not a power of two. Please
test it is > 0 explicitly here: I don't think this splitter will work
correctly otherwise. There shouldn't really be a shift by 0 ever of
course, but it isn't invalid RTL.
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr105991.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +unsigned long long
> +foo (unsigned long long value)
> +{
> + value &= 0xffffffff;
> + value |= value << 32;
> + return value;
> +}
> +/* { dg-final { scan-assembler "rldimi" } } */
Write
/* { dg-final { scan-assembler {\mrldimi\M} } } */
please.
Okay for trunk with those changes. Thanks!
Segher
