On Tue, Jun 23, 2026 at 6:42 AM H.J. Lu <[email protected]> wrote:
>
> On Mon, Jun 22, 2026 at 9:32 PM H.J. Lu <[email protected]> wrote:
> >
> > On Mon, Jun 22, 2026 at 7:50 PM Uros Bizjak <[email protected]> wrote:
> > >
> > > On Mon, Jun 22, 2026 at 1:29 PM Uros Bizjak <[email protected]> wrote:
> > > >
> > > > On Mon, Jun 22, 2026 at 8:33 AM H.J. Lu <[email protected]> wrote:
> > > > >
> > > > > On Mon, Jun 22, 2026 at 1:59 PM Uros Bizjak <[email protected]> wrote:
> > > > > >
> > > > > > -ENOPATCH
> > > > >
> > > > > Oops.  Here it is.
> > > > >
> > > > > > On Sun, Jun 21, 2026 at 11:44 PM H.J. Lu <[email protected]> 
> > > > > > wrote:
> > > > > > >
> > > > > > > Since LCP stall peepholes are added after register allocation, 
> > > > > > > each
> > > > > > > peephole may use a different scratch register.  For input:
> > > > > > >
> > > > > > > extern void bar (void);
> > > > > > >
> > > > > > > void
> > > > > > > foo (short *dst)
> > > > > > > {
> > > > > > >   dst[0] = 3;
> > > > > > >   asm volatile ("" : : : "memory");
> > > > > > >   dst[2] = 3;
> > > > > > >   bar ();
> > > > > > >   dst[1] = 3;
> > > > > > >   asm volatile ("" : : : "memory");
> > > > > > >   dst[4] = 3;
> > > > > > > }
> > > > > > >
> > > > > > > with LCP stall peepholes, GCC generates:
> > > > > > >
> > > > > > > movl $3, %eax
> > > > > > > pushq %rbx
> > > > > > > movq %rdi, %rbx
> > > > > > > movw %ax, (%rdi)
> > > > > > > movl $3, %edx
> > > > > > > movw %dx, 4(%rdi)
> > > > > > > call bar
> > > > > > > movl $3, %ecx
> > > > > > > movw %cx, 2(%rbx)
> > > > > > > movl $3, %esi
> > > > > > > movw %si, 8(%rbx)
> > > > > > > popq %rbx
> > > > > > >
> > > > > > > using 4 different scratch registers vs without LCP stall 
> > > > > > > peepholes:
> > > > > > >
> > > > > > > pushq %rbx
> > > > > > > movq %rdi, %rbx
> > > > > > > movw $3, (%rdi)
> > > > > > > movw $3, 4(%rdi)
> > > > > > > call bar
> > > > > > > movw $3, 2(%rbx)
> > > > > > > movw $3, 8(%rbx)
> > > > > > > popq %rbx
> > > > > > >
> > > > > > > Add ix86_output_lcp_stall_peephole to generate LCP stall 
> > > > > > > peepholes with
> > > > > > > the previous scratch register:
> > > > > > >
> > > > > > > 1. Scan backward for the previous scratch register definition with
> > > > > > > the same immediate operand in the same basic block.
> > > > > > > 2. The previous scratch register is unusable if it is set between 
> > > > > > > the
> > > > > > > previous scratch register definition and the current instruction.
> > > > > > > 3. If a usable previous scratch register is found, ignore the 
> > > > > > > allocated
> > > > > > > scratch register and use the previous scratch register.  
> > > > > > > Otherwise, use
> > > > > > > the allocated scratch register.
> > > > > > >
> > > > > > > so that the same scratch register can be reused if possible:
> > > > > > >
> > > > > > > movl $3, %eax
> > > > > > > pushq %rbx
> > > > > > > movq %rdi, %rbx
> > > > > > > movw %ax, (%rdi)
> > > > > > > movw %ax, 4(%rdi)
> > > > > > > call bar
> > > > > > > movl $3, %ecx
> > > > > > > movw %cx, 2(%rbx)
> > > > > > > movw %cx, 8(%rbx)
> > > > > > > popq %rbx
> > > > > > >
> > > > > > > I backported this patch to GCC 16:
> > > > > > >
> > > > > > > 1. When bootstrapping GCC 16 with only C and C++ enabled, this 
> > > > > > > optimization
> > > > > > > triggers 54 times.  No regressions.
> > > > > > > 2. When building glibc 2.44, this optimization triggers 33 times. 
> > > > > > >  No
> > > > > > > regressions.
> > > > > > > 3. When building Linux kernel 7.1.1, this optimization triggers 
> > > > > > > 2099 times.
> > > > > > > Kernel boots correctly.
> > > > > > >
> > > > > > > gcc/
> > > > > > >
> > > > > > > PR target/125893
> > > > > > > * config/i386/i386-protos.h (ix86_output_lcp_stall_peephole):
> > > > > > > New.
> > > > > > > * config/i386/i386.cc (ix86_output_lcp_stall_peephole): Likewise.
> > > > > > > * config/i386/i386.md (TARGET_LCP_STALL peepholes): Call
> > > > > > > ix86_output_lcp_stall_peephole.
> > > > > > >
> > > > > > > gcc/testsuite/
> > > > > > >
> > > > > > > PR target/125893
> > > > > > > * gcc.target/i386/pr125893-1.c: New test.
> > > > > > > * gcc.target/i386/pr125893-2.c: Likewise.
> > > > > > > * gcc.target/i386/pr125893-3.c: Likewise.
> > > > > > > * gcc.target/i386/pr125893-4.c: Likewise.
> > > > > > > * gcc.target/i386/pr125893-5.c: Likewise.
> > > > > > > * gcc.target/i386/pr125893-6.c: Likewise.
> > > > > > > * gcc.target/i386/pr125893-7.c: Likewise.
> > > > > > > * gcc.target/i386/pr125893-8.c: Likewise.
> > > > > > > * gcc.target/i386/pr125893-9.c: Likewise.
> > > > > > > * gcc.target/i386/pr125893-10.c: Likewise.
> > > >
> > > >
> > > > +/* Output LCP stall or long immediate peephole for INSN.  Use the
> > > > +   previous scratch register if possible.  */
> > > > +
> > > > +void
> > > > +ix86_output_lcp_stall_peephole (rtx_insn *insn, rtx *operands)
> > > > +{
> > > >
> > > > Please name this function ix86_expand_lcp_stall_peephole. We are
> > > > expanding RTXes here, not outputting ASM.
> >
> > Will fix
>
> Done in the v2 patch.
>
> >
> > > > -  [(parallel [(set (match_dup 2) (const_int 0))
> > > > -          (clobber (reg:CC FLAGS_REG))])
> > > > -   (set (match_dup 0) (match_dup 1))]
> > > > -  "operands[2] = gen_lowpart (SImode, operands[1]);")
> > > > +  [(const_int 0)]
> > > > +{
> > > > +  operands[2] = const0_rtx;
> > > > +  ix86_output_lcp_stall_peephole (curr_insn, operands);
> > > > +})
> > > >
> > > > Missing DONE;
> >
> > Will fix
>
> Done in the v2 patch.
>
> >
> > > > -  [(set (match_dup 2) (match_dup 1))
> > > > -   (set (match_dup 0) (match_dup 2))])
> > > > +  [(const_int 0)]
> > > > +  "ix86_output_lcp_stall_peephole (curr_insn, operands);")
> > > >
> > > > Also here.
> >
> > Will fix
>
> Done in the v2 patch.
>
> >
> > > > On a related note, the second peephole also allows const0_rtx, which
> > > > results in XOR form, which clobbers flags. Please better use some
> > > > boolean argument (e.g. use_xor) to the ix86_output_lcp_stall_peephole
> > > > and handle it accordingly in both peephole2 patterns.
> >
> > Will fix
>
> Done in the v2 patch.
>
> > > Another observation: You can't reuse %esi/%edi/%ebp/%esp as QImode
> > > scratch in 32-bit mode. So, the value, available in one of these
> > > registers, can not be reused as QImode value. Please re-test the patch
> >
> > Will fix.
>
> Added
>
>                  if (!TARGET_64BIT && mode == QImode)
>                     {
>                       /* SI_REG/DI_REG/BP_REG/SP_REG has no Qimode in
>                          32-bit mode.  */
>                       unsigned int regno = REGNO (dest);
>                       if (regno >= SI_REG && regno <= SP_REG)
>                         continue;
>                     }

if (mode == QImode && !ANY_QI_REGNO_P (REGNO (dest)))
    continue;

>
> together with a testcase, pr125893-11.c, extracted from i686 glibc build.
>
> > > also for 32-bit mode.
> > >
>
> Also tested with Linux/x86-64 and Linux/i686 glibc builds.
>
> --
> H.J.
> ---
> Since LCP stall peepholes are added after register allocation, each
> peephole may use a different scratch register.  For input:
>
> extern void bar (void);
>
> void
> foo (short *dst)
> {
>   dst[0] = 3;
>   asm volatile ("" : : : "memory");
>   dst[2] = 3;
>   bar ();
>   dst[1] = 3;
>   asm volatile ("" : : : "memory");
>   dst[4] = 3;
> }
>
> with LCP stall peepholes, GCC generates:
>
> movl $3, %eax
> pushq %rbx
> movq %rdi, %rbx
> movw %ax, (%rdi)
> movl $3, %edx
> movw %dx, 4(%rdi)
> call bar
> movl $3, %ecx
> movw %cx, 2(%rbx)
> movl $3, %esi
> movw %si, 8(%rbx)
> popq %rbx
>
> using 4 different scratch registers vs without LCP stall peepholes:
>
> pushq %rbx
> movq %rdi, %rbx
> movw $3, (%rdi)
> movw $3, 4(%rdi)
> call bar
> movw $3, 2(%rbx)
> movw $3, 8(%rbx)
> popq %rbx
>
> Add ix86_output_lcp_stall_peephole to generate LCP stall peepholes with
> the previous scratch register:
>
> 1. Scan backward for the previous scratch register definition with
> the same immediate operand in the same basic block.
> 2. The previous scratch register is unusable if it is set between the
> previous scratch register definition and the current instruction.
> 3. If a usable previous scratch register is found, ignore the allocated
> scratch register and use the previous scratch register.  Otherwise, use
> the allocated scratch register.
>
> so that the same scratch register can be reused if possible:
>
> movl $3, %eax
> pushq %rbx
> movq %rdi, %rbx
> movw %ax, (%rdi)
> movw %ax, 4(%rdi)
> call bar
> movl $3, %ecx
> movw %cx, 2(%rbx)
> movw %cx, 8(%rbx)
> popq %rbx
>
> I backported this patch to GCC 16:
>
> 1. When bootstrapping GCC 16 with only C and C++ enabled, this optimization
> triggers 54 times.  No regressions.
> 2. When building glibc 2.44, this optimization triggers 33 times.  No
> regressions.
> 3. When building Linux kernel 7.1.1, this optimization triggers 2099 times.
> Kernel boots correctly.
>
> Tested on Linux/x86-64 and Linux/i686.
>
> gcc/
>
> PR target/125893
> * config/i386/i386-expand.cc (ix86_expand_lcp_stall_peephole): New.
> * config/i386/i386-protos.h (ix86_expand_lcp_stall_peephole):
> Likewise.
> * config/i386/i386.md (TARGET_LCP_STALL peepholes): Call
> ix86_expand_lcp_stall_peephole.
>
> gcc/testsuite/
>
> PR target/125893
> * gcc.target/i386/pr125893-1.c: New test.
> * gcc.target/i386/pr125893-2.c: Likewise.
> * gcc.target/i386/pr125893-3.c: Likewise.
> * gcc.target/i386/pr125893-4.c: Likewise.
> * gcc.target/i386/pr125893-5.c: Likewise.
> * gcc.target/i386/pr125893-6.c: Likewise.
> * gcc.target/i386/pr125893-7.c: Likewise.
> * gcc.target/i386/pr125893-8.c: Likewise.
> * gcc.target/i386/pr125893-9.c: Likewise.
> * gcc.target/i386/pr125893-10.c: Likewise.
> * gcc.target/i386/pr125893-11.c: Likewise.

+  rtx dest = SET_DEST (set);
+  /* The previous scratch register mode must be at least
+     MODE.  */
+  if (!REG_P (dest)
+      || (GET_MODE_SIZE (GET_MODE (dest))
+  < GET_MODE_SIZE (mode)))
+    continue;

With improvement to QImode test, both tests can collapse to:

rtx dest = SET_DEST (set);

/* Reject DEST if a register is not wide enough to supply MODE
    or invalid for QImode. */
if (!REG_P (dest)
    || GET_MODE_SIZE (GET_MODE (dest)) < GET_MODE_SIZE (mode)
    || (mode == QImode && !ANY_QI_REGNO_P (REGNO (dest))))
  continue;

OK with the above change.

Thanks,
Uros.

Reply via email to