Skip to content

Commit b3cb0c3

Browse files
ubizjakubizjak
committed
i386: Fix addcarry/subborrow issues [PR117860]
Fix several things to enable combine to handle addcarry/subborrow patterns: - Fix wrong canonical form of addcarry<mode> insn and friends. For commutative operand (PLUS RTX) binary operand (LTU) takes precedence before unary operand (ZERO_EXTEND). - Swap operands of GTU comparison to canonicalize addcarry/subborrow comparison. Again, the canonical form of the compare is PLUS RTX before ZERO_EXTEND RTX. GTU comparison is not a carry flag comparison, so we have to swap operands in x86_canonicalize_comparison to a non-canonical form to use LTU comparison. - Return correct compare mode (CCCmode) for addcarry/subborrow pattern from ix86_cc_mode, so combine is able to emit required compare mode for combined insn. - Add *subborrow<mode>_1 pattern having const_scalar_int_operand predicate. Here, canonicalization of SUB (op1, const) RTX to PLUS (op1, -const) requires negation of constant operand when ckecking operands. With the above changes, combine is able to create *addcarry_1/*subborrow_1 pattern with immediate operand for the testcase in the PR: SomeAddFunc: addq %rcx, %rsi # 10 [c=4 l=3] adddi3_cc_overflow_1/0 movq %rdi, %rax # 33 [c=4 l=3] *movdi_internal/3 adcq $5, %rdx # 19 [c=4 l=4] *addcarrydi_1/0 movq %rsi, (%rdi) # 23 [c=4 l=3] *movdi_internal/5 movq %rdx, 8(%rdi) # 24 [c=4 l=4] *movdi_internal/5 setc %dl # 39 [c=4 l=3] *setcc_qi movzbl %dl, %edx # 40 [c=4 l=3] zero_extendqidi2/0 movq %rdx, 16(%rdi) # 26 [c=4 l=4] *movdi_internal/5 ret # 43 [c=0 l=1] simple_return_internal SomeSubFunc: subq %rcx, %rsi # 10 [c=4 l=3] *subdi_3/0 movq %rdi, %rax # 42 [c=4 l=3] *movdi_internal/3 sbbq $17, %rdx # 19 [c=4 l=4] *subborrowdi_1/0 movq %rsi, (%rdi) # 33 [c=4 l=3] *movdi_internal/5 sbbq %rcx, %rcx # 29 [c=8 l=3] *x86_movdicc_0_m1_neg movq %rdx, 8(%rdi) # 34 [c=4 l=4] *movdi_internal/5 movq %rcx, 16(%rdi) # 35 [c=4 l=4] *movdi_internal/5 ret # 51 [c=0 l=1] simple_return_internal PR target/117860 gcc/ChangeLog: * config/i386/i386.cc (ix86_canonicalize_comparison): Swap operands of GTU comparison to canonicalize addcarry/subborrow comparison. (ix86_cc_mode): Return CCCmode for the comparison of addcarry/subborrow pattern. * config/i386/i386.md (addcarry<mode>): Swap operands of PLUS RTX to make it canonical. (*addcarry<mode>_1): Ditto. (addcarry peephole2s): Update RTXes for addcarry<mode>_1 change. (*add<dwi>3_doubleword_cc_overflow_1): Ditto. (*subborrow<mode>_1): New insn pattern. gcc/testsuite/ChangeLog: * gcc.target/i386/pr117860.c: New test.
1 parent a92b2be commit b3cb0c3

File tree

3 files changed

+140
-20
lines changed

3 files changed

+140
-20
lines changed

‎gcc/config/i386/i386.cc‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,11 +578,25 @@ ix86_canonicalize_comparison (int *code, rtx *op0, rtx *op1,
578578
{
579579
std::swap (*op0, *op1);
580580
*code = (int) scode;
581+
return;
581582
}
582583
}
584+
585+
/* Swap operands of GTU comparison to canonicalize
586+
addcarry/subborrow comparison. */
587+
if (!op0_preserve_value
588+
&& *code == GTU
589+
&& GET_CODE (*op0) == PLUS
590+
&& ix86_carry_flag_operator (XEXP (*op0, 0), VOIDmode)
591+
&& GET_CODE (XEXP (*op0, 1)) == ZERO_EXTEND
592+
&& GET_CODE (*op1) == ZERO_EXTEND)
593+
{
594+
std::swap (*op0, *op1);
595+
*code = (int) swap_condition ((enum rtx_code) *code);
596+
return;
597+
}
583598
}
584599

585-
586600
/* Hook to determine if one function can safely inline another. */
587601

588602
static bool
@@ -16479,6 +16493,13 @@ ix86_cc_mode (enum rtx_code code, rtx op0, rtx op1)
1647916493
&& GET_CODE (op1) == GEU
1648016494
&& GET_MODE (XEXP (op1, 0)) == CCCmode)
1648116495
return CCCmode;
16496+
/* Similarly for the comparison of addcarry/subborrow pattern. */
16497+
else if (code == LTU
16498+
&& GET_CODE (op0) == ZERO_EXTEND
16499+
&& GET_CODE (op1) == PLUS
16500+
&& ix86_carry_flag_operator (XEXP (op1, 0), VOIDmode)
16501+
&& GET_CODE (XEXP (op1, 1)) == ZERO_EXTEND)
16502+
return CCCmode;
1648216503
else
1648316504
return CCmode;
1648416505
case GTU: /* CF=0 & ZF=0 */

‎gcc/config/i386/i386.md‎

Lines changed: 66 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9036,12 +9036,12 @@
90369036
(match_operand:SWI48 1 "nonimmediate_operand" "%0,0,rm,r"))
90379037
(match_operand:SWI48 2 "nonimmediate_operand" "r,rm,r,m")))
90389038
(plus:<DWI>
9039-
(zero_extend:<DWI> (match_dup 2))
90409039
(match_operator:<DWI> 4 "ix86_carry_flag_operator"
9041-
[(match_dup 3) (const_int 0)]))))
9040+
[(match_dup 3) (const_int 0)])
9041+
(zero_extend:<DWI> (match_dup 2)))))
90429042
(set (match_operand:SWI48 0 "nonimmediate_operand" "=rm,r,r,r")
90439043
(plus:SWI48 (plus:SWI48 (match_op_dup 5
9044-
[(match_dup 3) (const_int 0)])
9044+
[(match_dup 3) (const_int 0)])
90459045
(match_dup 1))
90469046
(match_dup 2)))]
90479047
"ix86_binary_operator_ok (PLUS, <MODE>mode, operands, TARGET_APX_NDD)"
@@ -9068,9 +9068,9 @@
90689068
(match_operand:SWI48 0 "general_reg_operand"))
90699069
(match_operand:SWI48 1 "memory_operand")))
90709070
(plus:<DWI>
9071-
(zero_extend:<DWI> (match_dup 1))
90729071
(match_operator:<DWI> 3 "ix86_carry_flag_operator"
9073-
[(match_dup 2) (const_int 0)]))))
9072+
[(match_dup 2) (const_int 0)])
9073+
(zero_extend:<DWI> (match_dup 1)))))
90749074
(set (match_dup 0)
90759075
(plus:SWI48 (plus:SWI48 (match_op_dup 4
90769076
[(match_dup 2) (const_int 0)])
@@ -9090,9 +9090,9 @@
90909090
(match_dup 1))
90919091
(match_dup 0)))
90929092
(plus:<DWI>
9093-
(zero_extend:<DWI> (match_dup 0))
90949093
(match_op_dup 3
9095-
[(match_dup 2) (const_int 0)]))))
9094+
[(match_dup 2) (const_int 0)])
9095+
(zero_extend:<DWI> (match_dup 0)))))
90969096
(set (match_dup 1)
90979097
(plus:SWI48 (plus:SWI48 (match_op_dup 4
90989098
[(match_dup 2) (const_int 0)])
@@ -9113,9 +9113,9 @@
91139113
(match_dup 0))
91149114
(match_operand:SWI48 2 "memory_operand")))
91159115
(plus:<DWI>
9116-
(zero_extend:<DWI> (match_dup 2))
91179116
(match_operator:<DWI> 4 "ix86_carry_flag_operator"
9118-
[(match_dup 3) (const_int 0)]))))
9117+
[(match_dup 3) (const_int 0)])
9118+
(zero_extend:<DWI> (match_dup 2)))))
91199119
(set (match_dup 0)
91209120
(plus:SWI48 (plus:SWI48 (match_op_dup 5
91219121
[(match_dup 3) (const_int 0)])
@@ -9137,9 +9137,9 @@
91379137
(match_dup 1))
91389138
(match_dup 0)))
91399139
(plus:<DWI>
9140-
(zero_extend:<DWI> (match_dup 0))
91419140
(match_op_dup 4
9142-
[(match_dup 3) (const_int 0)]))))
9141+
[(match_dup 3) (const_int 0)])
9142+
(zero_extend:<DWI> (match_dup 0)))))
91439143
(set (match_dup 1)
91449144
(plus:SWI48 (plus:SWI48 (match_op_dup 5
91459145
[(match_dup 3) (const_int 0)])
@@ -9158,9 +9158,9 @@
91589158
(match_operand:SWI48 0 "general_reg_operand"))
91599159
(match_operand:SWI48 1 "memory_operand")))
91609160
(plus:<DWI>
9161-
(zero_extend:<DWI> (match_dup 1))
91629161
(match_operator:<DWI> 3 "ix86_carry_flag_operator"
9163-
[(match_dup 2) (const_int 0)]))))
9162+
[(match_dup 2) (const_int 0)])
9163+
(zero_extend:<DWI> (match_dup 1)))))
91649164
(set (match_dup 0)
91659165
(plus:SWI48 (plus:SWI48 (match_op_dup 4
91669166
[(match_dup 2) (const_int 0)])
@@ -9188,9 +9188,9 @@
91889188
(match_dup 1))
91899189
(match_dup 0)))
91909190
(plus:<DWI>
9191-
(zero_extend:<DWI> (match_dup 0))
91929191
(match_op_dup 3
9193-
[(match_dup 2) (const_int 0)]))))
9192+
[(match_dup 2) (const_int 0)])
9193+
(zero_extend:<DWI> (match_dup 0)))))
91949194
(set (match_dup 1)
91959195
(plus:SWI48 (plus:SWI48 (match_op_dup 4
91969196
[(match_dup 2) (const_int 0)])
@@ -9222,9 +9222,9 @@
92229222
(match_operand:SWI48 1 "nonimmediate_operand" "%0,rm"))
92239223
(match_operand:SWI48 2 "x86_64_immediate_operand" "e,e")))
92249224
(plus:<DWI>
9225-
(match_operand:<DWI> 6 "const_scalar_int_operand")
92269225
(match_operator:<DWI> 4 "ix86_carry_flag_operator"
9227-
[(match_dup 3) (const_int 0)]))))
9226+
[(match_dup 3) (const_int 0)])
9227+
(match_operand:<DWI> 6 "const_scalar_int_operand"))))
92289228
(set (match_operand:SWI48 0 "nonimmediate_operand" "=rm,r")
92299229
(plus:SWI48 (plus:SWI48 (match_op_dup 5
92309230
[(match_dup 3) (const_int 0)])
@@ -9748,6 +9748,53 @@
97489748
(minus:SWI48 (match_dup 1) (match_dup 2)))])]
97499749
"ix86_binary_operator_ok (MINUS, <MODE>mode, operands, TARGET_APX_NDD)")
97509750

9751+
(define_insn "*subborrow<mode>_1"
9752+
[(set (reg:CCC FLAGS_REG)
9753+
(compare:CCC
9754+
(zero_extend:<DWI>
9755+
(match_operand:SWI48 1 "nonimmediate_operand" "0,rm"))
9756+
(plus:<DWI>
9757+
(match_operator:<DWI> 4 "ix86_carry_flag_operator"
9758+
[(match_operand 3 "flags_reg_operand") (const_int 0)])
9759+
(match_operand:<DWI> 6 "const_scalar_int_operand"))))
9760+
(set (match_operand:SWI48 0 "nonimmediate_operand" "=rm,r")
9761+
(plus:SWI48 (minus:SWI48
9762+
(match_dup 1)
9763+
(match_operator:SWI48 5 "ix86_carry_flag_operator"
9764+
[(match_dup 3) (const_int 0)]))
9765+
(match_operand:SWI48 2 "x86_64_immediate_operand" "e,e")))]
9766+
"ix86_binary_operator_ok (MINUS, <MODE>mode, operands, TARGET_APX_NDD)
9767+
&& CONST_INT_P (operands[2])
9768+
/* Check that operands[6] is -operands[2] zero extended from
9769+
<MODE>mode to <DWI>mode. */
9770+
&& ((<MODE>mode == SImode || -INTVAL (operands[2]) >= 0)
9771+
? (CONST_INT_P (operands[6])
9772+
&& (UINTVAL (operands[6])
9773+
== ((unsigned HOST_WIDE_INT) -INTVAL (operands[2])
9774+
& GET_MODE_MASK (<MODE>mode))))
9775+
: (CONST_WIDE_INT_P (operands[6])
9776+
&& CONST_WIDE_INT_NUNITS (operands[6]) == 2
9777+
&& ((unsigned HOST_WIDE_INT) CONST_WIDE_INT_ELT (operands[6], 0)
9778+
== (unsigned HOST_WIDE_INT) -INTVAL (operands[2]))
9779+
&& CONST_WIDE_INT_ELT (operands[6], 1) == 0))"
9780+
{
9781+
bool use_ndd = get_attr_isa (insn) == ISA_APX_NDD;
9782+
9783+
operands[2] = GEN_INT (-INTVAL (operands[2]));
9784+
9785+
return use_ndd ? "sbb{<imodesuffix>}\t{%2, %1, %0|%0, %1, %2}"
9786+
: "sbb{<imodesuffix>}\t{%2, %0|%0, %2}";
9787+
}
9788+
[(set_attr "isa" "*,apx_ndd")
9789+
(set_attr "type" "alu")
9790+
(set_attr "use_carry" "1")
9791+
(set_attr "pent_pair" "pu")
9792+
(set_attr "mode" "<MODE>")
9793+
(set (attr "length_immediate")
9794+
(if_then_else (match_test "IN_RANGE (-INTVAL (operands[2]), -128, 127)")
9795+
(const_string "1")
9796+
(const_string "4")))])
9797+
97519798
(define_expand "uaddc<mode>5"
97529799
[(match_operand:SWI48 0 "register_operand")
97539800
(match_operand:SWI48 1 "register_operand")
@@ -10040,8 +10087,8 @@
1004010087
(match_dup 4))
1004110088
(match_dup 5)))
1004210089
(plus:<DWI>
10043-
(match_dup 6)
10044-
(ltu:<DWI> (reg:CC FLAGS_REG) (const_int 0)))))
10090+
(ltu:<DWI> (reg:CC FLAGS_REG) (const_int 0))
10091+
(match_dup 6))))
1004510092
(set (match_dup 3)
1004610093
(plus:DWIH
1004710094
(plus:DWIH (ltu:DWIH (reg:CC FLAGS_REG) (const_int 0))

‎gcc/testsuite/gcc.target/i386/pr117860.c‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/* PR target/117116 */
2+
/* { dg-do compile { target { ! ia32 } } } */
3+
/* { dg-options "-O2 -masm=att" } */
4+
5+
#include <stdint.h>
6+
7+
#if (defined(__GNUC__) || defined(__clang__))
8+
#include <immintrin.h>
9+
#elif defined(_MSC_VER)
10+
#include <intrin.h>
11+
#endif
12+
13+
typedef struct {
14+
uint64_t lo64;
15+
uint64_t mid64;
16+
uint64_t hi64;
17+
} UInt192;
18+
19+
UInt192 SomeAddFunc(uint64_t a_lo, uint64_t a_hi, uint64_t b) {
20+
UInt192 result;
21+
unsigned char cf;
22+
unsigned long long sum;
23+
24+
cf = _addcarry_u64(0, a_lo, b, &sum);
25+
result.lo64 = sum;
26+
27+
cf = _addcarry_u64(cf, a_hi, 5, &sum);
28+
result.mid64 = sum;
29+
result.hi64 = cf;
30+
31+
return result;
32+
}
33+
34+
/* { dg-final { scan-assembler "adcq\[ \\t\]+\\\$5," } } */
35+
36+
UInt192 SomeSubFunc(uint64_t a_lo, uint64_t a_hi, uint64_t b) {
37+
UInt192 result;
38+
unsigned char cf;
39+
unsigned long long diff;
40+
41+
cf = _subborrow_u64(0, a_lo, b, &diff);
42+
result.lo64 = diff;
43+
44+
cf = _subborrow_u64(cf, a_hi, 17, &diff);
45+
result.mid64 = diff;
46+
(void)_subborrow_u64(cf, 0, 0, &diff);
47+
result.hi64 = diff;
48+
49+
return result;
50+
}
51+
52+
/* { dg-final { scan-assembler "sbbq\[ \\t\]+\\\$17," } } */

0 commit comments

Comments
 (0)