From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id A7BAB3858D1E for ; Wed, 14 Jun 2023 12:25:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A7BAB3858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id C870622533; Wed, 14 Jun 2023 12:25:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1686745546; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=T1yDxs+P2zpQLpOGeJnxX3AYvnZv+BJXYhXc99GP/DY=; b=Mw9mB77bfU9KDGOPMpkDhwQ4s4TlKFlhG+6vkwjd3ZRA1xICzgv+ummOoRzWukUSRVaKQ7 HzgL6CaQ4UCq6UOJ8tm2lbnYoL1106HOrsBEDngaQeSSAh/WxPliMsZFbKteCtb0eyuVHj I0IZ8/ZgfZ7A4kN3G3mq7+QdpxdK1Wk= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1686745546; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=T1yDxs+P2zpQLpOGeJnxX3AYvnZv+BJXYhXc99GP/DY=; b=WctEobd8pyGgNRgj6l1BK9rTbUb0cjKcVNKOatqvs5TzNzUdatpwjPoopbdkx9kNXAe2E8 +pp3s4K3d4JG5SDQ== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id B7A382C141; Wed, 14 Jun 2023 12:25:46 +0000 (UTC) Date: Wed, 14 Jun 2023 12:25:46 +0000 (UTC) From: Richard Biener To: Jakub Jelinek cc: Uros Bizjak , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] middle-end, i386: Pattern recognize add/subtract with carry [PR79173] In-Reply-To: Message-ID: References: User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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.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) == COMPLEX_TYPE ? TREE_TYPE (type) : type; > + bool ovf = false; > + tree r = int_const_binop (subcode, fold_convert (itype, arg0), > + fold_convert (itype, arg1)); > + if (!r || TREE_CODE (r) != INTEGER_CST) > + return NULL_TREE; > + if (arith_overflowed_p (subcode, itype, arg0, arg1)) > + ovf = true; > + if (TREE_OVERFLOW (r)) > + r = drop_tree_overflow (r); > + if (itype == type) > + { > + if (ovf) > + return NULL_TREE; > + return r; > + } > + else > + return build_complex (type, r, build_int_cst (itype, ovf)); > + } > + return NULL_TREE; > + > default: > return fold_const_call_1 (fn, type, arg0, arg1); > } > @@ -1896,6 +1937,30 @@ fold_const_call (combined_fn fn, tree ty > return NULL_TREE; > } > > + case CFN_UADDC: > + case CFN_USUBC: > + if (integer_cst_p (arg0) && integer_cst_p (arg1) && integer_cst_p (arg2)) > + { > + tree itype = TREE_TYPE (type); > + bool ovf = false; > + tree_code subcode = fn == CFN_UADDC ? PLUS_EXPR : MINUS_EXPR; > + tree r = int_const_binop (subcode, fold_convert (itype, arg0), > + fold_convert (itype, arg1)); > + if (!r) > + return NULL_TREE; > + if (arith_overflowed_p (subcode, itype, arg0, arg1)) > + ovf = true; > + tree r2 = int_const_binop (subcode, r, fold_convert (itype, arg2)); > + if (!r2 || TREE_CODE (r2) != INTEGER_CST) > + return NULL_TREE; > + if (arith_overflowed_p (subcode, itype, r, arg2)) > + ovf = true; > + if (TREE_OVERFLOW (r2)) > + r2 = drop_tree_overflow (r2); > + return build_complex (type, r2, build_int_cst (itype, ovf)); > + } > + return NULL_TREE; > + > default: > return fold_const_call_1 (fn, type, arg0, arg1, arg2); > } > > > Jakub > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)