From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 4401E385559C for ; Thu, 25 May 2023 12:36:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4401E385559C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 00C1C1042; Thu, 25 May 2023 05:37:26 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 4F8543F762; Thu, 25 May 2023 05:36:40 -0700 (PDT) From: Richard Sandiford To: juzhe.zhong@rivai.ai Mail-Followup-To: juzhe.zhong@rivai.ai,gcc-patches@gcc.gnu.org, rguenther@suse.de, richard.sandiford@arm.com Cc: gcc-patches@gcc.gnu.org, rguenther@suse.de Subject: Re: [PATCH V16] VECT: Add decrement IV iteration loop control by variable amount support References: <20230525120550.6275-1-juzhe.zhong@rivai.ai> Date: Thu, 25 May 2023 13:36:39 +0100 In-Reply-To: <20230525120550.6275-1-juzhe.zhong@rivai.ai> (juzhe zhong's message of "Thu, 25 May 2023 20:05:50 +0800") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-22.5 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: This looks good to me. Just a couple of very minor cosmetic things: juzhe.zhong@rivai.ai writes: > @@ -753,17 +846,35 @@ vect_set_loop_condition_partial_vectors (class loop *loop, > continue; > } > > - /* See whether zero-based IV would ever generate all-false masks > - or zero length before wrapping around. */ > - bool might_wrap_p = vect_rgroup_iv_might_wrap_p (loop_vinfo, rgc); > - > - /* Set up all controls for this group. */ > - test_ctrl = vect_set_loop_controls_directly (loop, loop_vinfo, > - &preheader_seq, > - &header_seq, > - loop_cond_gsi, rgc, > - niters, niters_skip, > - might_wrap_p); > + if (!LOOP_VINFO_USING_DECREMENTING_IV_P (loop_vinfo) || !iv_rgc > + || (iv_rgc->max_nscalars_per_iter * iv_rgc->factor > + != rgc->max_nscalars_per_iter * rgc->factor)) Coding style is to put each subcondition on a separate line when the whole condition doesn't fit on a single line. So: if (!LOOP_VINFO_USING_DECREMENTING_IV_P (loop_vinfo) || !iv_rgc || (iv_rgc->max_nscalars_per_iter * iv_rgc->factor != rgc->max_nscalars_per_iter * rgc->factor)) > @@ -2725,6 +2726,17 @@ start_over: > && !vect_verify_loop_lens (loop_vinfo)) > LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo) = false; > > + /* If we're vectorizing an loop that uses length "controls" and s/an loop/a loop/ (Sorry for not noticing earlier.) OK for trunk from my POV with those changes; no need to repost unless your policies require it. Please give Richi a chance to comment too though. Thanks for your patience with the review process. The final result seems pretty clean to me. Richard