public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug inline-asm/67317] New: [x86] Silly code generation for _addcarry_u32/_addcarry_u64
@ 2015-08-22  4:59 myriachan at gmail dot com
  2015-08-22  6:14 ` [Bug inline-asm/67317] " pinskia at gcc dot gnu.org
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: myriachan at gmail dot com @ 2015-08-22  4:59 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 67317
           Summary: [x86] Silly code generation for
                    _addcarry_u32/_addcarry_u64
           Product: gcc
           Version: 5.2.0
            Status: UNCONFIRMED
          Severity: minor
          Priority: P3
         Component: inline-asm
          Assignee: unassigned at gcc dot gnu.org
          Reporter: myriachan at gmail dot com
  Target Milestone: ---

x86 intrinsics _addcarry_u32 and _addcarry_u64 generate silly code.  For
example, the following function to get the result of a 64-bit addition (the XOR
is to the output clearer):

        u64 testcarry(u64 a, u64 b, u64 c, u64 d)
        {
                u64 result0, result1;
                _addcarry_u64(_addcarry_u64(0, a, c, &result0), b, d,
&result1);
                return result0 ^ result1;
        }

This is the code generated with -O1, -O2 and -O3:

        xor     r8d, r8d
        add     r8b, -1
        adc     rdx, rdi
        setc    r8b
        mov     rax, rdx
        add     r8b, -1
        adc     rcx, rsi
        xor     rax, rcx
        ret

The first sillyness is that _addcarry_u64 does not optimize a compile-time
constant 0 being the first carry parameter.  Instead of "adc", it should just
use "add".

The second sillyness is with the use of r8b to store the carry flag, then using
"add r8b, -1" to put the result back into carry.

Instead, the code should be something like this:

        add     rdx, rdi
        mov     rax, rdx
        adc     rcx, rsi
        xor     rax, rcx
        ret

Naturally, for something this simple, I'd use unsigned __int128, but this came
up in large number math.


^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-09-02 15:08 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-22  4:59 [Bug inline-asm/67317] New: [x86] Silly code generation for _addcarry_u32/_addcarry_u64 myriachan at gmail dot com
2015-08-22  6:14 ` [Bug inline-asm/67317] " pinskia at gcc dot gnu.org
2015-08-25  8:24 ` rguenth at gcc dot gnu.org
2015-08-25 11:10 ` [Bug target/67317] " glisse at gcc dot gnu.org
2015-08-25 12:58 ` segher at gcc dot gnu.org
2015-08-25 16:48 ` ubizjak at gmail dot com
2015-08-25 19:26 ` segher at gcc dot gnu.org
2015-08-27  8:53 ` ubizjak at gmail dot com
2015-08-27 18:30 ` uros at gcc dot gnu.org
2015-09-02 15:07 ` uros at gcc dot gnu.org
2015-09-02 15:08 ` ubizjak at gmail dot com

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).