[Bug target/93990] [x86] Silly code generation for _addcarry_u32/_addcarry_u64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93990 Andrew Pinski changed: What|Removed |Added Status|REOPENED|NEW Severity|normal |enhancement --- Comment #5 from Andrew Pinski --- _addcarry_u64(0, a, c, &result0) can definitely be folded into __builtin_uadd_overflowll/.ADD_OVERFLOW without any troubles. The second _addcarry_u64 here since the result is not used can be folded into just an a few adds. That leaves if there case where the first argument is non-zero and the result is used. I think going the route that Jakub mentions is best for this last case.
[Bug target/93990] [x86] Silly code generation for _addcarry_u32/_addcarry_u64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93990 --- Comment #6 from Andrew Pinski --- (In reply to Andrew Pinski from comment #5) > That leaves if there case where the first argument is non-zero and the > result is used. I think going the route that Jakub mentions is best for this > last case. Maybe even generic builtins/internal functions for this case even as clang has __builtin_addc, etc.
[Bug target/93990] [x86] Silly code generation for _addcarry_u32/_addcarry_u64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93990 --- Comment #1 from Andrew Pinski --- This is already been fixed in GCC 10: addq%rdx, %rdi movq%rsi, %rax adcq%rcx, %rax xorq%rdi, %rax ret .ident "GCC: (GNU) 10.0.1 20200121 (experimental) [master revision e0a5b313c1a:f4dc220f630:b313d3c49c2387b5e212df22a5e6ecc0c4e95c0a]"
[Bug target/93990] [x86] Silly code generation for _addcarry_u32/_addcarry_u64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93990 Jack Lloyd changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #2 from Jack Lloyd --- Good enough for me, closing, thanks.
[Bug target/93990] [x86] Silly code generation for _addcarry_u32/_addcarry_u64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93990 Jakub Jelinek changed: What|Removed |Added Status|RESOLVED|REOPENED Last reconfirmed||2020-03-03 CC||jakub at gcc dot gnu.org Resolution|FIXED |--- Ever confirmed|0 |1 --- Comment #3 from Jakub Jelinek --- I must say I can't really reproduce this, tried with -O3 r227270 (there is expected worse code), r227271 (already good), r23, r24, r25, r26, r270002, r28, r10-6985 and all emit the same 4 instructions (addq, adcq, movq and xorq) + ret, just the scheduling slightly varries over time. Though, looking at your assembly, seems you are using -fstack-protector-all or -fstack-protector-strong, with that option it indeed emits more code, but it did already in r227271. And the reason why we get worse code with one of -fstack-protector-{all,strong} is that the stores by the builtin are done using alias set 0 and so RTL DSE isn't able to remove the stores because it thinks they might alias with the stack canary. One way to fix this would be I'd say to use alias set of unsigned int for unsigned long long (depending on what the builtin prototype is), and another one, perhaps better long term, would be fold those builtins during gimple to some another md builtin, perhaps with space in name to make it user unaccessible, which would return a complex unsigned or complex unsigned long long containing both results, so that we wouldn't have the addressable variable during expansion at all and stack protector wouldn't need to mark anything (unless there are other user vars that need it).
[Bug target/93990] [x86] Silly code generation for _addcarry_u32/_addcarry_u64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93990 --- Comment #4 from Jakub Jelinek --- I have tried: --- gcc/config/i386/i386-expand.c.jj2020-01-12 11:54:36.323414766 +0100 +++ gcc/config/i386/i386-expand.c 2020-03-03 12:44:54.116134173 +0100 @@ -11902,7 +11902,18 @@ rdseed_step: emit_insn (gen_rtx_SET (target, pat)); /* Store the result. */ - emit_move_insn (gen_rtx_MEM (mode0, op4), op0); + { +op4 = gen_rtx_MEM (mode0, op4); +tree type = (mode0 == SImode ? unsigned_type_node +: long_long_unsigned_type_node); + if (TREE_CODE (arg3) == ADDR_EXPR + && useless_type_conversion_p (TREE_TYPE (TREE_OPERAND (arg3, 0)), + type)) + set_mem_attributes (op4, TREE_OPERAND (arg3, 0), 0); + else + set_mem_attributes (op4, type, 0); + emit_move_insn (op4, op0); + } return target; but even that doesn't help, the stores now have [1 result0+0 S8 A64] instead of former [0 S8 A8], but RTL DSE still doesn't remove those for whatever reason.