https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113560

Uroš Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

--- Comment #11 from Uroš Bizjak <ubizjak at gmail dot com> ---
My recent patch [1] "regressed" asm scan patterns from this PR. As stated in
the commit message:

    The second multiplication in g++.target/i386/pr113560.C needs only the
    high part and now uses MULX instead of MULQ, and the register
    allocator no longer needs to preserve %rax across it, so the return
    value increment becomes ADDQ instead of LEAQ.

But instead of just changing scan patterns, the tree dump of
pr113560.C.272t.optimized exposes deeper issue: 

__attribute__((returns_nonnull))
char * f (char * buf, long long unsigned int in)
{
  __int128 unsigned __res;
  unsigned short _1;
  char * _6;
  __int128 unsigned _9;
  unsigned long _10;
  unsigned long highparttmp_13;

  <bb 2> [local count: 1073741824]:
  __res_12 = in_3(D) w* 184467440738;
  _10 = (unsigned long) __res_12;
  highparttmp_13 = _10 h* 100;
  _9 = (__int128 unsigned) highparttmp_13;
  _1 = (unsigned short) _9;
  MEM <unsigned short> [(char * {ref-all})buf_4(D) + 2B] = _1;
  _6 = buf_4(D) + 10;
  return _6;

}

Here,

  __int128 unsigned __res;
  unsigned long _10;

  __res_12 = in_3(D) w* 184467440738;
  _10 = (unsigned long) __res_12;

There is no need for widening multiplication, because only lowpart of the
result is used. Even before my patch, the compiler generates:

_Z1fPcy:
        movabsq $184467440738, %rdx     # 34    [c=5 l=10]  *movdi_internal/4
        movq    %rdi, %rcx      # 21    [c=4 l=3]  *movdi_internal/3
        movl    $100, %eax      # 36    [c=4 l=5]  *movdi_internal/2
        mulx    %rsi, %rsi, %rdi        # 35    [c=0 l=5]  *bmi2_umulditi3_1
        mulq    %rsi    # 37    [c=20 l=3]  umuldi3_highpart
        leaq    10(%rcx), %rax  # 27    [c=4 l=4]  *leadi
        movw    %dx, 2(%rcx)    # 12    [c=4 l=4]  *movhi_internal/3
        ret             # 30    [c=0 l=1]  simple_return_internal

where widening MULX is generated insted of IMULQ. Clang gets it right:

_Z1fPcy:                                # @_Z1fPcy
        movabsq $184467440738, %rdx             # imm = 0x2AF31DC462
        imulq   %rsi, %rdx
        movl    $100, %eax
        mulxq   %rax, %rax, %rax
        movw    %ax, 2(%rdi)
        leaq    10(%rdi), %rax
        retq

Please note that non-widening IMULQ has much more freedom w.r.t. operand
allocation than MULQ or even MULX and should be preferred. After my patch, MULX
is generated for umuldi3_highpart, but unnecessarily widening *bmi2_umulditi3_1
remains.

Conversion from a widening mul to a non-widening mul (as now exposed by
g++.target/i386/pr113560.C) is so important for x86 that I decided to reopen
this PR and leave the testsuite "regression" unfixed ATM.

[1] https://gcc.gnu.org/pipermail/gcc-patches/2026-October/735147.html

Reply via email to