On Thu, Jul 9, 2026 at 6:25 PM Roger Sayle <[email protected]> wrote: > > > > -----Original Message----- > > From: Richard Biener <[email protected]> > > Sent: 08 July 2026 13:10 > > To: Roger Sayle <[email protected]> > > Cc: GCC Patches <[email protected]> > > Subject: Re: [PATCH] PR target/48609: Improve RTL expansion of complex value > > return. > > > > On Wed, Jul 1, 2026 at 1:07 PM Roger Sayle <[email protected]> > > wrote: > > > This patch address the inefficient return of complex values (with the > > > x86 > > > ABI) where the result is returned to the caller in an integer register. > > > Currently this results in RTL expansion spilling the value to memory > > > and reloading it in an integer register. The patch below recognizes > > > this case, and composes the real and imaginary parts using shifts and > > > addition. The real part always appears first in memory, so is lowpart > > > on little-endian targets, and the highpart on big-endian targets. > > > > > > Consider the new test case: > > > > > > _Complex float mem; > > > _Complex float foo(_Complex float x) { return x; } _Complex float > > > bar() { return mem; } > > > > > > Currently, with -O2 GCC generates: > > > > > > foo: movss %xmm0, -8(%rsp) > > > shufps $85, %xmm0, %xmm0 > > > movss %xmm0, -4(%rsp) > > > movq -8(%rsp), %xmm0 > > > ret > > > > > > bar: movss mem(%rip), %xmm0 > > > movss %xmm0, -8(%rsp) > > > movss mem+4(%rip), %xmm0 > > > movss %xmm0, -4(%rsp) > > > movq -8(%rsp), %xmm0 > > > ret > > > > > > With this patch, we now generate: > > > > > > foo: ret > > > > > > bar: movl mem+4(%rip), %edx > > > movl mem(%rip), %eax > > > salq $32, %rdx > > > addq %rdx, %rax > > > movq %rax, %xmm0 > > > ret > > > > > > > > > For those folks noticing that bar could be improved further, I've a > > > follow-up patch to the i386's STV2 pass, to perform concatsidi2 in SSE > > > registers. > > > > > > 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? > > > > Seeing a GET_CODE (src) == CONCAT case after the new block I wonder if > > that's > > more specific and should be handled first? > > No. The CONCAT case after this block is more generic (handling modes > other than complex modes) and is where the inefficiency lies, by calling > assign_stack_temp to always force this value to the stack. > > > Or would that be always worse when SCALAR_INT_MODE_P (mode) in > > which case we possibly want to add a comment reflecting this. > > It's shouldn't need a comment. It comes at this position in the > sequential if-then-else chain because the transformations that > appear earlier (should) take priority, and this transformation has > priority over the transformations that appear after it. > > > Can expand_shift fail? > > Not for scalar integer modes. For LSHIFT_EXPR, the middle-end > will even fall back to repeated additions, for targets without optabs. > > > I would expect it might be quite expensive, like on AVR which IIRC > > can only shift by a single bit at a time. > > Here we're shifting by the size of a complex number's component, > typically 32, 64 or 16 bits. AVR and similar microcontrollers have > special cases in their shift expansion to efficiently handle shifts by > multiples of bytes. > > More significantly, the use of CONCAT in emit_group_load is to > support the requirements of the target's ABI, and it's only broken > ABIs (such as x86 and x86_64) that mandate passing complex > values packed into a single 64-bit integers. I've confirmed that > code generation on avr-elf is unaffected by this change. > > Hypothetically, if there is a target where spilling a value to > memory and reloading it was cheaper/more efficient than > the no-op "shift and any_or_plus" pair, the backend (now) has > the opportunity to lower it, by creating the stack slot itself. > The problem with the current code, is that it mandates a > stack spill whether the backend wants one or not. > > > That said, I wonder why emit_group_load_1 is the correct place to fix? > > is it that the very CONCAT path performs assign_stack_temp ()? > > Yes, it's the CONCAT path's assign_stack_temp that is the problem. > > This problem is local to the RTL expansion pass, specifically > emit_group_load_1. Before this, the reads/writes to memory don't > exist and aren't visible to the tree-ssa optimizers. After this, the > RTL optimizers are unable to eliminate these reads and writes to > the stack. > > > I'm happy to answer your questions. Let me know if there's > anything else you don't understand or are worried about. > It's easy to forget that if the solution was obvious, someone > would probably have fixed this (and similar issues) by now. > PR 48609 is over 15 years old!
Thanks for answering, the patch is OK. Richard. > > It's stage 1. Getting a solution into the tree, provides > much more thorough testing than fears over what-ifs. > > Thanks in advance, > Roger > -- > > > > 2026-07-01 Roger Sayle <[email protected]> > > > > > > gcc/ChangeLog > > > PR target/48609 > > > * expr.cc (emit_group_load_1): When passing a complex value in an > > > integer mode of the same size, explicitly construct (hi<<N)+lo to > > > avoid spilling to memory before reload. > > > > > > gcc/testsuite/ChangeLog > > > PR target/48609 > > > * gcc.target/i386/pr48609-2.c: New test case. > > > > > > Thanks in advance, > > > Roger > > > -- >
