From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id D3D4C3857C44 for ; Thu, 6 May 2021 01:37:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D3D4C3857C44 Received: from pps.filterd (m0127361.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 1461Yagr076743; Wed, 5 May 2021 21:37:03 -0400 Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 38c0q9rgcj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 05 May 2021 21:37:03 -0400 Received: from m0127361.ppops.net (m0127361.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.43/8.16.0.43) with SMTP id 1461ZLZD078066; Wed, 5 May 2021 21:37:02 -0400 Received: from ppma02dal.us.ibm.com (a.bd.3ea9.ip4.static.sl-reverse.com [169.62.189.10]) by mx0a-001b2d01.pphosted.com with ESMTP id 38c0q9rgcd-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 05 May 2021 21:37:02 -0400 Received: from pps.filterd (ppma02dal.us.ibm.com [127.0.0.1]) by ppma02dal.us.ibm.com (8.16.0.43/8.16.0.43) with SMTP id 1461S1HJ021722; Thu, 6 May 2021 01:37:02 GMT Received: from b01cxnp23034.gho.pok.ibm.com (b01cxnp23034.gho.pok.ibm.com [9.57.198.29]) by ppma02dal.us.ibm.com with ESMTP id 38c1mxt8qw-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 06 May 2021 01:37:02 +0000 Received: from b01ledav002.gho.pok.ibm.com (b01ledav002.gho.pok.ibm.com [9.57.199.107]) by b01cxnp23034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1461b1X528901774 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 6 May 2021 01:37:01 GMT Received: from b01ledav002.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 6FDD1124053; Thu, 6 May 2021 01:37:01 +0000 (GMT) Received: from b01ledav002.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 23323124054; Thu, 6 May 2021 01:37:01 +0000 (GMT) Received: from ltc.linux.ibm.com (unknown [9.10.229.42]) by b01ledav002.gho.pok.ibm.com (Postfix) with ESMTP; Thu, 6 May 2021 01:37:01 +0000 (GMT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Thu, 06 May 2021 09:37:00 +0800 From: guojiufu To: Segher Boessenkool Cc: gcc-patches@gcc.gnu.org, wschmidt@linux.ibm.com, dje.gcc@gmail.com, rguenther@suse.de, jlaw@tachyum.com Subject: Re: [PATCH] split loop for NE condition. In-Reply-To: <20210430213723.GC10366@gate.crashing.org> References: <20210429095048.76239-1-guojiufu@linux.ibm.com> <20210430213723.GC10366@gate.crashing.org> Message-ID: <0b3676d0c9aa71a9c4b306528734abb1@imap.linux.ibm.com> X-Sender: guojiufu@linux.ibm.com User-Agent: Roundcube Webmail/1.1.12 X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: oaiTwXef2B0tsSwwjbA3-Tb6cfHI56Q4 X-Proofpoint-GUID: IYtXPx1Ty90mnIOcstncRO-Hxqv2K-tq X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391, 18.0.761 definitions=2021-05-05_11:2021-05-05, 2021-05-05 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 spamscore=0 clxscore=1015 mlxscore=0 adultscore=0 phishscore=0 suspectscore=0 mlxlogscore=999 impostorscore=0 lowpriorityscore=0 bulkscore=0 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104060000 definitions=main-2105060008 X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 May 2021 01:37:05 -0000 On 2021-05-01 05:37, Segher Boessenkool wrote: > Hi! > > On Thu, Apr 29, 2021 at 05:50:48PM +0800, Jiufu Guo wrote: >> When there is the possibility that overflow may happen on the loop >> index, >> a few optimizations would not happen. For example code: >> >> foo (int *a, int *b, unsigned k, unsigned n) >> { >> while (++k != n) >> a[k] = b[k] + 1; >> } >> >> For this code, if "l > n", overflow may happen. if "l < n" at >> begining, >> it could be optimized (e.g. vectorization). > > FWIW, this isn't called "overflow" in C: all overflow is undefined > behaviour. > > "A computation involving unsigned operands can never overflow, because > a > result that cannot be represented by the resulting unsigned integer > type > is reduced modulo the number that is one greater than the largest value > that can be represented by the resulting type." Thanks for point out this, yes, it may be better to call it as 'wrap' :) > >> +-param=max-insns-ne-cond-split= >> +Common Joined UInteger Var(param_max_insn_ne_cond_split) Init(64) >> Param Optimization >> +The maximum threshold for insnstructions number of a loop with ne >> condition to split. > > "number of instructions". > > Perhaps you should mark up "ne" as a codeword somehow, but because it > is in a help text it is probably better to just, write out "not equal" > or similar? Would update it accordingly. Thanks for your suggestion! > >> @@ -248,13 +250,14 @@ connect_loop_phis (class loop *loop1, class loop >> *loop2, edge new_e) >> !gsi_end_p (psi_first); >> gsi_next (&psi_first), gsi_next (&psi_second)) >> { >> - tree init, next, new_init; >> + tree init, next, new_init, prev; >> use_operand_p op; >> gphi *phi_first = psi_first.phi (); >> gphi *phi_second = psi_second.phi (); >> >> init = PHI_ARG_DEF_FROM_EDGE (phi_first, firste); >> next = PHI_ARG_DEF_FROM_EDGE (phi_first, firstn); >> + prev = PHI_RESULT (phi_first); >> op = PHI_ARG_DEF_PTR_FROM_EDGE (phi_second, seconde); >> gcc_assert (operand_equal_for_phi_arg_p (init, USE_FROM_PTR >> (op))); >> > > I would just declare it at the first use... Less mental load for the > reader. (And a smaller patch ;-) ) Yeap, thanks! > >> +/* Check if the LOOP exit branch likes "if (idx != bound)". >> + if INV is not NULL and the branch is "if (bound != idx)", set *INV >> to true. > > "If INV", sentences start with a capital. Thanks :) > >> + /* Make sure idx and bound. */ >> + tree idx = gimple_cond_lhs (cond); >> + tree bnd = gimple_cond_rhs (cond); >> + if (expr_invariant_in_loop_p (loop, idx)) >> + { >> + std::swap (idx, bnd); >> + if (inv) >> + *inv = true; >> + } >> + else if (!expr_invariant_in_loop_p (loop, bnd)) >> + continue; > > Make sure idx and bound what? What about them? > >> + /* Make sure idx is iv. */ >> + class loop *useloop = loop_containing_stmt (cond); >> + affine_iv iv; >> + if (!simple_iv (loop, useloop, idx, &iv, false)) >> + continue; > > "Make sure idx is a simple_iv"? Thanks, the comment should be more clear, the intention is: make sure "lhs/rhs" pair are "index/bound" pair. > >> + >> + /* No need to split loop, if base is know value. >> + Or check range info. */ > > "if base is a known value". Not sure what you mean with range info? > A possible future improvement? The intention is "If there is no wrap/overflow happen", no need to split loop". If the base is a known value, the index may not wrap/overflow and may be able optimized by other passes. Using range-info to check wrap/overflow could be a future improvement. > >> + /* There is type conversion on idx(or rhs of idx's def). >> + And there is converting shorter to longer type. */ >> + tree type = TREE_TYPE (idx); >> + if (!INTEGRAL_TYPE_P (type) || TREE_CODE (idx) != SSA_NAME >> + || !TYPE_UNSIGNED (type) >> + || TYPE_PRECISION (type) == TYPE_PRECISION (sizetype)) >> + continue; > > "IDX is an unsigned type that is widened to SIZETYPE" etc. This is better wording :) > > This code assumes SIZETYPE is bigger than any other integer type. Is > that true? Even if so, the second comment could be improved. > > (Not reviewing further, my Gimple isn't near good enough, sorry. But > at least to my untrained eye it looks pretty good :-) ) Thanks so much for your very helpful comments! Jiufu Guo. > > > Segher