Re: [PATCH] middle-end, i386: Pattern recognize add/subtract with carry [PR79173]
On Tue, 13 Jun 2023, Jakub Jelinek wrote:
> On Tue, Jun 13, 2023 at 08:40:36AM +, Richard Biener wrote:
> > I suspect re-association can wreck things even more here. I have
> > to say the matching code is very hard to follow, not sure if
> > splitting out a function matching
> >
> >_22 = .{ADD,SUB}_OVERFLOW (_6, _5);
> >_23 = REALPART_EXPR <_22>;
> >_24 = IMAGPART_EXPR <_22>;
> >
> > from _23 and _24 would help?
>
> I've outlined 3 most often used sequences of statements or checks
> into 3 helper functions, hope that helps.
>
> > > + while (TREE_CODE (rhs[0]) == SSA_NAME && !rhs[3])
> > > + {
> > > + gimple *g = SSA_NAME_DEF_STMT (rhs[0]);
> > > + if (has_single_use (rhs[0])
> > > + && is_gimple_assign (g)
> > > + && (gimple_assign_rhs_code (g) == code
> > > + || (code == MINUS_EXPR
> > > + && gimple_assign_rhs_code (g) == PLUS_EXPR
> > > + && TREE_CODE (gimple_assign_rhs2 (g)) == INTEGER_CST)))
> > > + {
> > > + rhs[0] = gimple_assign_rhs1 (g);
> > > + tree = rhs[2] ? rhs[3] : rhs[2];
> > > + r = gimple_assign_rhs2 (g);
> > > + if (gimple_assign_rhs_code (g) != code)
> > > + r = fold_build1 (NEGATE_EXPR, TREE_TYPE (r), r);
> >
> > Can you use const_unop here? In fact both will not reliably
> > negate all constants (ick), so maybe we want a force_const_negate ()?
>
> It is unsigned type NEGATE_EXPR of INTEGER_CST, so I think it should
> work. That said, changed it to const_unop and am just giving up on it
> as if it wasn't a PLUS_EXPR with INTEGER_CST addend if const_unop doesn't
> simplify.
>
> > > + else if (addc_subc)
> > > + {
> > > + if (!integer_zerop (arg2))
> > > + ;
> > > + /* x = y + 0 + 0; x = y - 0 - 0; */
> > > + else if (integer_zerop (arg1))
> > > + result = arg0;
> > > + /* x = 0 + y + 0; */
> > > + else if (subcode != MINUS_EXPR && integer_zerop (arg0))
> > > + result = arg1;
> > > + /* x = y - y - 0; */
> > > + else if (subcode == MINUS_EXPR
> > > +&& operand_equal_p (arg0, arg1, 0))
> > > + result = integer_zero_node;
> > > + }
> >
> > So this all performs simplifications but also constant folding. In
> > particular the match.pd re-simplification will invoke fold_const_call
> > on all-constant argument function calls but does not do extra folding
> > on partially constant arg cases but instead relies on patterns here.
> >
> > Can you add all-constant arg handling to fold_const_call and
> > consider moving cases like y + 0 + 0 to match.pd?
>
> The reason I've done this here is that this is the spot where all other
> similar internal functions are handled, be it the ubsan ones
> - IFN_UBSAN_CHECK_{ADD,SUB,MUL}, or __builtin_*_overflow ones
> - IFN_{ADD,SUB,MUL}_OVERFLOW, or these 2 new ones. The code handles
> there 2 constant arguments as well as various patterns that can be
> simplified and has code to clean it up later, build a COMPLEX_CST,
> or COMPLEX_EXPR etc. as needed. So, I think we want to handle those
> elsewhere, we should do it for all of those functions, but then
> probably incrementally.
>
> > > +@cindex @code{addc@var{m}5} instruction pattern
> > > +@item @samp{addc@var{m}5}
> > > +Adds operands 2, 3 and 4 (where the last operand is guaranteed to have
> > > +only values 0 or 1) together, sets operand 0 to the result of the
> > > +addition of the 3 operands and sets operand 1 to 1 iff there was no
> > > +overflow on the unsigned additions, and to 0 otherwise. So, it is
> > > +an addition with carry in (operand 4) and carry out (operand 1).
> > > +All operands have the same mode.
> >
> > operand 1 set to 1 for no overflow sounds weird when specifying it
> > as carry out - can you double check?
>
> Fixed.
>
> > > +@cindex @code{subc@var{m}5} instruction pattern
> > > +@item @samp{subc@var{m}5}
> > > +Similarly to @samp{addc@var{m}5}, except subtracts operands 3 and 4
> > > +from operand 2 instead of adding them. So, it is
> > > +a subtraction with carry/borrow in (operand 4) and carry/borrow out
> > > +(operand 1). All operands have the same mode.
> > > +
> >
> > I wonder if we want to name them uaddc and usubc? Or is this supposed
> > to be simply the twos-complement "carry"? I think the docs should
> > say so then (note we do have uaddv and addv).
>
> Makes sense, I've actually renamed even the internal functions etc.
>
> Here is only lightly tested patch with everything but gimple-fold.cc
> changed.
>
> 2023-06-13 Jakub Jelinek
>
> PR middle-end/79173
> * internal-fn.def (UADDC, USUBC): New internal functions.
> * internal-fn.cc (expand_UADDC, expand_USUBC): New functions.
> (commutative_ternary_fn_p): Return true also for IFN_UADDC.
> * optabs.def (uaddc5_optab, usubc5_optab): New optabs.
> * tree-ssa-math-opts.cc (uaddc_cast, uaddc_ne0, uaddc_is_cplxpart,
> match_uaddc_usubc): New
Re: [PATCH] middle-end, i386: Pattern recognize add/subtract with carry [PR79173]
On Wed, 14 Jun 2023, Jakub Jelinek wrote:
> On Tue, Jun 13, 2023 at 01:29:04PM +0200, Jakub Jelinek via Gcc-patches wrote:
> > > > + else if (addc_subc)
> > > > + {
> > > > + if (!integer_zerop (arg2))
> > > > + ;
> > > > + /* x = y + 0 + 0; x = y - 0 - 0; */
> > > > + else if (integer_zerop (arg1))
> > > > + result = arg0;
> > > > + /* x = 0 + y + 0; */
> > > > + else if (subcode != MINUS_EXPR && integer_zerop (arg0))
> > > > + result = arg1;
> > > > + /* x = y - y - 0; */
> > > > + else if (subcode == MINUS_EXPR
> > > > + && operand_equal_p (arg0, arg1, 0))
> > > > + result = integer_zero_node;
> > > > + }
> > >
> > > So this all performs simplifications but also constant folding. In
> > > particular the match.pd re-simplification will invoke fold_const_call
> > > on all-constant argument function calls but does not do extra folding
> > > on partially constant arg cases but instead relies on patterns here.
> > >
> > > Can you add all-constant arg handling to fold_const_call and
> > > consider moving cases like y + 0 + 0 to match.pd?
> >
> > The reason I've done this here is that this is the spot where all other
> > similar internal functions are handled, be it the ubsan ones
> > - IFN_UBSAN_CHECK_{ADD,SUB,MUL}, or __builtin_*_overflow ones
> > - IFN_{ADD,SUB,MUL}_OVERFLOW, or these 2 new ones. The code handles
> > there 2 constant arguments as well as various patterns that can be
> > simplified and has code to clean it up later, build a COMPLEX_CST,
> > or COMPLEX_EXPR etc. as needed. So, I think we want to handle those
> > elsewhere, we should do it for all of those functions, but then
> > probably incrementally.
>
> The patch I've posted yesterday now fully tested on x86_64-linux and
> i686-linux.
>
> Here is an untested incremental patch to handle constant folding of these
> in fold-const-call.cc rather than gimple-fold.cc.
> Not really sure if that is the way to go because it is replacing 28
> lines of former code with 65 of new code, for the overall benefit that say
> int
> foo (long long *p)
> {
> int one = 1;
> long long max = __LONG_LONG_MAX__;
> return __builtin_add_overflow (one, max, p);
> }
> can be now fully folded already in ccp1 pass while before it was only
> cleaned up in forwprop1 pass right after it.
I think that's still very much desirable so this followup looks OK.
Maybe you can re-base it as prerequesite though?
> As for doing some stuff in match.pd, I'm afraid it would result in even more
> significant growth, the advantage of gimple-fold.cc doing all of these in
> one place is that the needed infrastructure can be shared.
Yes, I saw that.
Richard.
>
> --- gcc/gimple-fold.cc.jj 2023-06-14 12:21:38.657657759 +0200
> +++ gcc/gimple-fold.cc2023-06-14 12:52:04.335054958 +0200
> @@ -5731,34 +5731,6 @@ gimple_fold_call (gimple_stmt_iterator *
> result = arg0;
> else if (subcode == MULT_EXPR && integer_onep (arg0))
> result = arg1;
> - if (type
> - && result == NULL_TREE
> - && TREE_CODE (arg0) == INTEGER_CST
> - && TREE_CODE (arg1) == INTEGER_CST
> - && (!uaddc_usubc || TREE_CODE (arg2) == INTEGER_CST))
> - {
> - if (cplx_result)
> - result = int_const_binop (subcode, fold_convert (type, arg0),
> - fold_convert (type, arg1));
> - else
> - result = int_const_binop (subcode, arg0, arg1);
> - if (result && arith_overflowed_p (subcode, type, arg0, arg1))
> - {
> - if (cplx_result)
> - overflow = build_one_cst (type);
> - else
> - result = NULL_TREE;
> - }
> - if (uaddc_usubc && result)
> - {
> - tree r = int_const_binop (subcode, result,
> - fold_convert (type, arg2));
> - if (r == NULL_TREE)
> - result = NULL_TREE;
> - else if (arith_overflowed_p (subcode, type, result, arg2))
> - overflow = build_one_cst (type);
> - }
> - }
> if (result)
> {
> if (result == integer_zero_node)
> --- gcc/fold-const-call.cc.jj 2023-06-02 10:36:43.096967505 +0200
> +++ gcc/fold-const-call.cc2023-06-14 12:56:08.195631214 +0200
> @@ -1669,6 +1669,7 @@ fold_const_call (combined_fn fn, tree ty
> {
>const char *p0, *p1;
>char c;
> + tree_code subcode;
>switch (fn)
> {
> case CFN_BUILT_IN_STRSPN:
> @@ -1738,6 +1739,46 @@ fold_const_call (combined_fn fn, tree ty
> case CFN_FOLD_LEFT_PLUS:
>return fold_const_fold_left (type, arg0, arg1, PLUS_EXPR);
>
> +case CFN_UBSAN_CHECK_ADD:
> +case CFN_ADD_OVERFLOW:
> +
Re: [PATCH] middle-end, i386: Pattern recognize add/subtract with carry [PR79173]
On Tue, Jun 13, 2023 at 01:29:04PM +0200, Jakub Jelinek via Gcc-patches wrote:
> > > + else if (addc_subc)
> > > + {
> > > + if (!integer_zerop (arg2))
> > > + ;
> > > + /* x = y + 0 + 0; x = y - 0 - 0; */
> > > + else if (integer_zerop (arg1))
> > > + result = arg0;
> > > + /* x = 0 + y + 0; */
> > > + else if (subcode != MINUS_EXPR && integer_zerop (arg0))
> > > + result = arg1;
> > > + /* x = y - y - 0; */
> > > + else if (subcode == MINUS_EXPR
> > > +&& operand_equal_p (arg0, arg1, 0))
> > > + result = integer_zero_node;
> > > + }
> >
> > So this all performs simplifications but also constant folding. In
> > particular the match.pd re-simplification will invoke fold_const_call
> > on all-constant argument function calls but does not do extra folding
> > on partially constant arg cases but instead relies on patterns here.
> >
> > Can you add all-constant arg handling to fold_const_call and
> > consider moving cases like y + 0 + 0 to match.pd?
>
> The reason I've done this here is that this is the spot where all other
> similar internal functions are handled, be it the ubsan ones
> - IFN_UBSAN_CHECK_{ADD,SUB,MUL}, or __builtin_*_overflow ones
> - IFN_{ADD,SUB,MUL}_OVERFLOW, or these 2 new ones. The code handles
> there 2 constant arguments as well as various patterns that can be
> simplified and has code to clean it up later, build a COMPLEX_CST,
> or COMPLEX_EXPR etc. as needed. So, I think we want to handle those
> elsewhere, we should do it for all of those functions, but then
> probably incrementally.
The patch I've posted yesterday now fully tested on x86_64-linux and
i686-linux.
Here is an untested incremental patch to handle constant folding of these
in fold-const-call.cc rather than gimple-fold.cc.
Not really sure if that is the way to go because it is replacing 28
lines of former code with 65 of new code, for the overall benefit that say
int
foo (long long *p)
{
int one = 1;
long long max = __LONG_LONG_MAX__;
return __builtin_add_overflow (one, max, p);
}
can be now fully folded already in ccp1 pass while before it was only
cleaned up in forwprop1 pass right after it.
As for doing some stuff in match.pd, I'm afraid it would result in even more
significant growth, the advantage of gimple-fold.cc doing all of these in
one place is that the needed infrastructure can be shared.
--- gcc/gimple-fold.cc.jj 2023-06-14 12:21:38.657657759 +0200
+++ gcc/gimple-fold.cc 2023-06-14 12:52:04.335054958 +0200
@@ -5731,34 +5731,6 @@ gimple_fold_call (gimple_stmt_iterator *
result = arg0;
else if (subcode == MULT_EXPR && integer_onep (arg0))
result = arg1;
- if (type
- && result == NULL_TREE
- && TREE_CODE (arg0) == INTEGER_CST
- && TREE_CODE (arg1) == INTEGER_CST
- && (!uaddc_usubc || TREE_CODE (arg2) == INTEGER_CST))
- {
- if (cplx_result)
- result = int_const_binop (subcode, fold_convert (type, arg0),
- fold_convert (type, arg1));
- else
- result = int_const_binop (subcode, arg0, arg1);
- if (result && arith_overflowed_p (subcode, type, arg0, arg1))
- {
- if (cplx_result)
- overflow = build_one_cst (type);
- else
- result = NULL_TREE;
- }
- if (uaddc_usubc && result)
- {
- tree r = int_const_binop (subcode, result,
- fold_convert (type, arg2));
- if (r == NULL_TREE)
- result = NULL_TREE;
- else if (arith_overflowed_p (subcode, type, result, arg2))
- overflow = build_one_cst (type);
- }
- }
if (result)
{
if (result == integer_zero_node)
--- gcc/fold-const-call.cc.jj 2023-06-02 10:36:43.096967505 +0200
+++ gcc/fold-const-call.cc 2023-06-14 12:56:08.195631214 +0200
@@ -1669,6 +1669,7 @@ fold_const_call (combined_fn fn, tree ty
{
const char *p0, *p1;
char c;
+ tree_code subcode;
switch (fn)
{
case CFN_BUILT_IN_STRSPN:
@@ -1738,6 +1739,46 @@ fold_const_call (combined_fn fn, tree ty
case CFN_FOLD_LEFT_PLUS:
return fold_const_fold_left (type, arg0, arg1, PLUS_EXPR);
+case CFN_UBSAN_CHECK_ADD:
+case CFN_ADD_OVERFLOW:
+ subcode = PLUS_EXPR;
+ goto arith_overflow;
+
+case CFN_UBSAN_CHECK_SUB:
+case CFN_SUB_OVERFLOW:
+ subcode = MINUS_EXPR;
+ goto arith_overflow;
+
+case CFN_UBSAN_CHECK_MUL:
+case CFN_MUL_OVERFLOW:
+ subcode = MULT_EXPR;
+ goto arith_overflow;
+
+arith_overflow:
+ if (integer_cst_p (arg0) && integer_cst_p (arg1))
+ {
+ tree itype
+ = TREE_CODE (type) ==
Re: [PATCH] middle-end, i386: Pattern recognize add/subtract with carry [PR79173]
On Tue, Jun 13, 2023 at 08:40:36AM +, Richard Biener wrote:
> I suspect re-association can wreck things even more here. I have
> to say the matching code is very hard to follow, not sure if
> splitting out a function matching
>
>_22 = .{ADD,SUB}_OVERFLOW (_6, _5);
>_23 = REALPART_EXPR <_22>;
>_24 = IMAGPART_EXPR <_22>;
>
> from _23 and _24 would help?
I've outlined 3 most often used sequences of statements or checks
into 3 helper functions, hope that helps.
> > + while (TREE_CODE (rhs[0]) == SSA_NAME && !rhs[3])
> > + {
> > + gimple *g = SSA_NAME_DEF_STMT (rhs[0]);
> > + if (has_single_use (rhs[0])
> > + && is_gimple_assign (g)
> > + && (gimple_assign_rhs_code (g) == code
> > + || (code == MINUS_EXPR
> > + && gimple_assign_rhs_code (g) == PLUS_EXPR
> > + && TREE_CODE (gimple_assign_rhs2 (g)) == INTEGER_CST)))
> > + {
> > + rhs[0] = gimple_assign_rhs1 (g);
> > + tree = rhs[2] ? rhs[3] : rhs[2];
> > + r = gimple_assign_rhs2 (g);
> > + if (gimple_assign_rhs_code (g) != code)
> > + r = fold_build1 (NEGATE_EXPR, TREE_TYPE (r), r);
>
> Can you use const_unop here? In fact both will not reliably
> negate all constants (ick), so maybe we want a force_const_negate ()?
It is unsigned type NEGATE_EXPR of INTEGER_CST, so I think it should
work. That said, changed it to const_unop and am just giving up on it
as if it wasn't a PLUS_EXPR with INTEGER_CST addend if const_unop doesn't
simplify.
> > + else if (addc_subc)
> > + {
> > + if (!integer_zerop (arg2))
> > + ;
> > + /* x = y + 0 + 0; x = y - 0 - 0; */
> > + else if (integer_zerop (arg1))
> > + result = arg0;
> > + /* x = 0 + y + 0; */
> > + else if (subcode != MINUS_EXPR && integer_zerop (arg0))
> > + result = arg1;
> > + /* x = y - y - 0; */
> > + else if (subcode == MINUS_EXPR
> > + && operand_equal_p (arg0, arg1, 0))
> > + result = integer_zero_node;
> > + }
>
> So this all performs simplifications but also constant folding. In
> particular the match.pd re-simplification will invoke fold_const_call
> on all-constant argument function calls but does not do extra folding
> on partially constant arg cases but instead relies on patterns here.
>
> Can you add all-constant arg handling to fold_const_call and
> consider moving cases like y + 0 + 0 to match.pd?
The reason I've done this here is that this is the spot where all other
similar internal functions are handled, be it the ubsan ones
- IFN_UBSAN_CHECK_{ADD,SUB,MUL}, or __builtin_*_overflow ones
- IFN_{ADD,SUB,MUL}_OVERFLOW, or these 2 new ones. The code handles
there 2 constant arguments as well as various patterns that can be
simplified and has code to clean it up later, build a COMPLEX_CST,
or COMPLEX_EXPR etc. as needed. So, I think we want to handle those
elsewhere, we should do it for all of those functions, but then
probably incrementally.
> > +@cindex @code{addc@var{m}5} instruction pattern
> > +@item @samp{addc@var{m}5}
> > +Adds operands 2, 3 and 4 (where the last operand is guaranteed to have
> > +only values 0 or 1) together, sets operand 0 to the result of the
> > +addition of the 3 operands and sets operand 1 to 1 iff there was no
> > +overflow on the unsigned additions, and to 0 otherwise. So, it is
> > +an addition with carry in (operand 4) and carry out (operand 1).
> > +All operands have the same mode.
>
> operand 1 set to 1 for no overflow sounds weird when specifying it
> as carry out - can you double check?
Fixed.
> > +@cindex @code{subc@var{m}5} instruction pattern
> > +@item @samp{subc@var{m}5}
> > +Similarly to @samp{addc@var{m}5}, except subtracts operands 3 and 4
> > +from operand 2 instead of adding them. So, it is
> > +a subtraction with carry/borrow in (operand 4) and carry/borrow out
> > +(operand 1). All operands have the same mode.
> > +
>
> I wonder if we want to name them uaddc and usubc? Or is this supposed
> to be simply the twos-complement "carry"? I think the docs should
> say so then (note we do have uaddv and addv).
Makes sense, I've actually renamed even the internal functions etc.
Here is only lightly tested patch with everything but gimple-fold.cc
changed.
2023-06-13 Jakub Jelinek
PR middle-end/79173
* internal-fn.def (UADDC, USUBC): New internal functions.
* internal-fn.cc (expand_UADDC, expand_USUBC): New functions.
(commutative_ternary_fn_p): Return true also for IFN_UADDC.
* optabs.def (uaddc5_optab, usubc5_optab): New optabs.
* tree-ssa-math-opts.cc (uaddc_cast, uaddc_ne0, uaddc_is_cplxpart,
match_uaddc_usubc): New functions.
(math_opts_dom_walker::after_dom_children): Call match_uaddc_usubc
for PLUS_EXPR, MINUS_EXPR, BIT_IOR_EXPR and BIT_XOR_EXPR unless
other optimizations have been
Re: [PATCH] middle-end, i386: Pattern recognize add/subtract with carry [PR79173]
On Tue, 6 Jun 2023, Jakub Jelinek wrote:
> Hi!
>
> The following patch introduces {add,sub}c5_optab and pattern recognizes
> various forms of add with carry and subtract with carry/borrow, see
> pr79173-{1,2,3,4,5,6}.c tests on what is matched.
> Primarily forms with 2 __builtin_add_overflow or __builtin_sub_overflow
> calls per limb (with just one for the least significant one), for
> add with carry even when it is hand written in C (for subtraction
> reassoc seems to change it too much so that the pattern recognition
> doesn't work). __builtin_{add,sub}_overflow are standardized in C23
> under ckd_{add,sub} names, so it isn't any longer a GNU only extension.
>
> Note, clang has for these has (IMHO badly designed)
> __builtin_{add,sub}c{b,s,,l,ll} builtins which don't add/subtract just
> a single bit of carry, but basically add 3 unsigned values or
> subtract 2 unsigned values from one, and result in carry out of 0, 1, or 2
> because of that. If we wanted to introduce those for clang compatibility,
> we could and lower them early to just two __builtin_{add,sub}_overflow
> calls and let the pattern matching in this patch recognize it later.
>
> I've added expanders for this on ix86 and in addition to that
> added various peephole2s to make sure we get nice (and small) code
> for the common cases. I think there are other PRs which request that
> e.g. for the _{addcarry,subborrow}_u{32,64} intrinsics, which the patch
> also improves.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Would be nice if support for these optabs was added to many other targets,
> arm/aarch64 and powerpc* certainly have such instructions, I'd expect
> in fact that most targets do.
>
> The _BitInt support I'm working on will also need this to emit reasonable
> code.
>
> 2023-06-06 Jakub Jelinek
>
> PR middle-end/79173
> * internal-fn.def (ADDC, SUBC): New internal functions.
> * internal-fn.cc (expand_ADDC, expand_SUBC): New functions.
> (commutative_ternary_fn_p): Return true also for IFN_ADDC.
> * optabs.def (addc5_optab, subc5_optab): New optabs.
> * tree-ssa-math-opts.cc (match_addc_subc): New function.
> (math_opts_dom_walker::after_dom_children): Call match_addc_subc
> for PLUS_EXPR, MINUS_EXPR, BIT_IOR_EXPR and BIT_XOR_EXPR unless
> other optimizations have been successful for those.
> * gimple-fold.cc (gimple_fold_call): Handle IFN_ADDC and IFN_SUBC.
> * gimple-range-fold.cc (adjust_imagpart_expr): Likewise.
> * tree-ssa-dce.cc (eliminate_unnecessary_stmts): Likewise.
> * doc/md.texi (addc5, subc5): Document new named
> patterns.
> * config/i386/i386.md (subborrow): Add alternative with
> memory destination.
> (addc5, subc5): New define_expand patterns.
> (*sub_3, @add3_carry, addcarry, @sub3_carry,
> subborrow, *add3_cc_overflow_1): Add define_peephole2
> TARGET_READ_MODIFY_WRITE/-Os patterns to prefer using memory
> destination in these patterns.
>
> * gcc.target/i386/pr79173-1.c: New test.
> * gcc.target/i386/pr79173-2.c: New test.
> * gcc.target/i386/pr79173-3.c: New test.
> * gcc.target/i386/pr79173-4.c: New test.
> * gcc.target/i386/pr79173-5.c: New test.
> * gcc.target/i386/pr79173-6.c: New test.
> * gcc.target/i386/pr79173-7.c: New test.
> * gcc.target/i386/pr79173-8.c: New test.
> * gcc.target/i386/pr79173-9.c: New test.
> * gcc.target/i386/pr79173-10.c: New test.
>
> --- gcc/internal-fn.def.jj2023-06-05 10:38:06.670333685 +0200
> +++ gcc/internal-fn.def 2023-06-05 11:40:50.672212265 +0200
> @@ -381,6 +381,8 @@ DEF_INTERNAL_FN (ASAN_POISON_USE, ECF_LE
> DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
> DEF_INTERNAL_FN (SUB_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
> DEF_INTERNAL_FN (MUL_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
> +DEF_INTERNAL_FN (ADDC, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
> +DEF_INTERNAL_FN (SUBC, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
> DEF_INTERNAL_FN (TSAN_FUNC_EXIT, ECF_NOVOPS | ECF_LEAF | ECF_NOTHROW, NULL)
> DEF_INTERNAL_FN (VA_ARG, ECF_NOTHROW | ECF_LEAF, NULL)
> DEF_INTERNAL_FN (VEC_CONVERT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
> --- gcc/internal-fn.cc.jj 2023-05-15 19:12:24.080780016 +0200
> +++ gcc/internal-fn.cc2023-06-06 09:38:46.333871169 +0200
> @@ -2722,6 +2722,44 @@ expand_MUL_OVERFLOW (internal_fn, gcall
>expand_arith_overflow (MULT_EXPR, stmt);
> }
>
> +/* Expand ADDC STMT. */
> +
> +static void
> +expand_ADDC (internal_fn ifn, gcall *stmt)
> +{
> + tree lhs = gimple_call_lhs (stmt);
> + tree arg1 = gimple_call_arg (stmt, 0);
> + tree arg2 = gimple_call_arg (stmt, 1);
> + tree arg3 = gimple_call_arg (stmt, 2);
> + tree type = TREE_TYPE (arg1);
> + machine_mode mode = TYPE_MODE (type);
> + insn_code icode = optab_handler (ifn == IFN_ADDC
> +
Re: Patch ping (Re: [PATCH] middle-end, i386: Pattern recognize add/subtract with carry [PR79173])
On Tue, Jun 13, 2023 at 9:06 AM Jakub Jelinek wrote:
>
> Hi!
>
> On Tue, Jun 06, 2023 at 11:42:07PM +0200, Jakub Jelinek via Gcc-patches wrote:
> > The following patch introduces {add,sub}c5_optab and pattern recognizes
> > various forms of add with carry and subtract with carry/borrow, see
> > pr79173-{1,2,3,4,5,6}.c tests on what is matched.
> > Primarily forms with 2 __builtin_add_overflow or __builtin_sub_overflow
> > calls per limb (with just one for the least significant one), for
> > add with carry even when it is hand written in C (for subtraction
> > reassoc seems to change it too much so that the pattern recognition
> > doesn't work). __builtin_{add,sub}_overflow are standardized in C23
> > under ckd_{add,sub} names, so it isn't any longer a GNU only extension.
> >
> > Note, clang has for these has (IMHO badly designed)
> > __builtin_{add,sub}c{b,s,,l,ll} builtins which don't add/subtract just
> > a single bit of carry, but basically add 3 unsigned values or
> > subtract 2 unsigned values from one, and result in carry out of 0, 1, or 2
> > because of that. If we wanted to introduce those for clang compatibility,
> > we could and lower them early to just two __builtin_{add,sub}_overflow
> > calls and let the pattern matching in this patch recognize it later.
> >
> > I've added expanders for this on ix86 and in addition to that
> > added various peephole2s to make sure we get nice (and small) code
> > for the common cases. I think there are other PRs which request that
> > e.g. for the _{addcarry,subborrow}_u{32,64} intrinsics, which the patch
> > also improves.
>
> I'd like to ping this patch.
I briefly went over the x86 part (LGTM), but please get a middle-end
approval first.
Thanks,
Uros.
>
> Thanks.
>
> Jakub
>
Patch ping (Re: [PATCH] middle-end, i386: Pattern recognize add/subtract with carry [PR79173])
Hi!
On Tue, Jun 06, 2023 at 11:42:07PM +0200, Jakub Jelinek via Gcc-patches wrote:
> The following patch introduces {add,sub}c5_optab and pattern recognizes
> various forms of add with carry and subtract with carry/borrow, see
> pr79173-{1,2,3,4,5,6}.c tests on what is matched.
> Primarily forms with 2 __builtin_add_overflow or __builtin_sub_overflow
> calls per limb (with just one for the least significant one), for
> add with carry even when it is hand written in C (for subtraction
> reassoc seems to change it too much so that the pattern recognition
> doesn't work). __builtin_{add,sub}_overflow are standardized in C23
> under ckd_{add,sub} names, so it isn't any longer a GNU only extension.
>
> Note, clang has for these has (IMHO badly designed)
> __builtin_{add,sub}c{b,s,,l,ll} builtins which don't add/subtract just
> a single bit of carry, but basically add 3 unsigned values or
> subtract 2 unsigned values from one, and result in carry out of 0, 1, or 2
> because of that. If we wanted to introduce those for clang compatibility,
> we could and lower them early to just two __builtin_{add,sub}_overflow
> calls and let the pattern matching in this patch recognize it later.
>
> I've added expanders for this on ix86 and in addition to that
> added various peephole2s to make sure we get nice (and small) code
> for the common cases. I think there are other PRs which request that
> e.g. for the _{addcarry,subborrow}_u{32,64} intrinsics, which the patch
> also improves.
I'd like to ping this patch.
Thanks.
Jakub
