On Fri, Jun 7, 2024 at 11:21 AM Roger Sayle <ro...@nextmovesoftware.com> wrote:
>
>
> This patch addresses PR target/115351, which is a code quality regression
> on x86 when passing floating point complex numbers. The ABI considers
> these arguments to have TImode, requiring interunit moves to place the
> FP values (which are actually passed in SSE registers) into the upper
> and lower parts of a TImode pseudo, and then similar moves back again
> before they can be used.
>
> The cause of the regression is that changes in how TImode initialization
> is represented in RTL now prevents the RTL optimizers from eliminating
> these redundant moves. The specific cause is that the *concatditi3
> pattern, (zext(hi)<<64)|zext(lo), has an inappropriately high (default)
> rtx_cost, preventing fwprop1 from propagating it. This pattern just
> sets the hipart and lopart of a double-word register, typically two
> instructions (less if reload can allocate things appropriately) but
> the current ix86_rtx_costs actually returns INSN_COSTS(13), i.e. 52.
>
> propagating insn 5 into insn 6, replacing:
> (set (reg:TI 110)
> (ior:TI (and:TI (reg:TI 110)
> (const_wide_int 0x0ffffffffffffffff))
> (ashift:TI (zero_extend:TI (subreg:DI (reg:DF 112 [ zD.2796+8 ]) 0))
> (const_int 64 [0x40]))))
> successfully matched this instruction to *concatditi3_3:
> (set (reg:TI 110)
> (ior:TI (ashift:TI (zero_extend:TI (subreg:DI (reg:DF 112 [ zD.2796+8 ])
> 0))
> (const_int 64 [0x40]))
> (zero_extend:TI (subreg:DI (reg:DF 111 [ zD.2796 ]) 0))))
> change not profitable (cost 50 -> cost 52)
>
> This issue is resolved by having ix86_rtx_costs return more reasonable
> values for these (place-holder) patterns.
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> and make -k check, both with and without --target_board=unix{-m32}
> with no new failures. Ok for mainline?
>
>
> 2024-06-07 Roger Sayle <ro...@nextmovesoftware.com>
>
> gcc/ChangeLog
> PR target/115351
> * config/i386/i386.cc (ix86_rtx_costs): Provide estimates for the
> *concatditi3 and *insvti_highpart patterns, about two insns.
>
> gcc/testsuite/ChangeLog
> PR target/115351
> * g++.target/i386/pr115351.C: New test case.
LGTM.
Thanks,
Uros.
>
>
> Thanks in advance (and sorry for any inconvenience),
> Roger
> --
>
