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 F31243857C59 for ; Fri, 17 Jul 2020 09:54:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org F31243857C59 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=richard.sandiford@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 B0722D6E; Fri, 17 Jul 2020 02:54:24 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C1AF53F66E; Fri, 17 Jul 2020 02:54:23 -0700 (PDT) From: Richard Sandiford To: "Kewen.Lin" Mail-Followup-To: "Kewen.Lin" , GCC Patches , Bill Schmidt , Richard Biener , Segher Boessenkool , dje.gcc@gmail.com, richard.sandiford@arm.com Cc: GCC Patches , Bill Schmidt , Richard Biener , Segher Boessenkool , dje.gcc@gmail.com Subject: Re: [PATCH 5/7 v7] vect: Support vector load/store with length in vectorizer References: <30906c0d-3b9f-e1e6-156f-c01fcf229cb9@linux.ibm.com> <4b7f2daf-467e-d940-b79c-31c1c30a1dd4@linux.ibm.com> <2fa0452b-9895-15f3-6db5-4233ff16b408@linux.ibm.com> <68981da8-f7a1-4c95-6d64-2a1d8b748b9f@linux.ibm.com> <5634e168-13fa-b870-3378-b78779793e1f@linux.ibm.com> <0eb2d1c1-f8e6-4f62-cc56-200228252650@linux.ibm.com> Date: Fri, 17 Jul 2020 10:54:22 +0100 In-Reply-To: (Kewen Lin's message of "Fri, 10 Jul 2020 17:55:56 +0800") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.5 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, 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: Fri, 17 Jul 2020 09:54:26 -0000 Hi, Sorry for the slow review. > The new version v7 is attached which has addressed your review comments > on v6. Could you have a further look? Many thanks in advance! > > Bootstrapped/regtested on aarch64-linux-gnu and powerpc64le-linux-gnu P9. > Even with explicit vect-partial-vector-usage settings 1/2 on Power target, > I didn't find any remarkable failures (only some trivial test case issues= ). Thanks, this looks great. OK for trunk with the minor nits below fixed. "Kewen.Lin" writes: > @@ -968,4 +968,8 @@ Bound on number of runtime checks inserted by the vec= torizer's loop versioning f > Common Joined UInteger Var(param_vect_max_version_for_alignment_checks) = Init(6) Param Optimization > Bound on number of runtime checks inserted by the vectorizer's loop vers= ioning for alignment check. >=20=20 > +-param=3Dvect-partial-vector-usage=3D > +Common Joined UInteger Var(param_vect_partial_vector_usage) Init(2) Inte= gerRange(0, 2) Param Optimization > +Controls how loop vectorizer uses partial vectors. 0 means never, 1 mea= ns only for loops whose iterating need can be removed, 2 means for all loop= s. The default value is 2. IMO reads better as s/iterating need/need to iterate/ > + FOR_EACH_MODE_IN_CLASS (tmode_iter, MODE_INT) > + { > + scalar_mode tmode =3D tmode_iter.require (); > + unsigned int tbits =3D GET_MODE_BITSIZE (tmode); > + > + /* ??? Do we really want to construct one IV whose precision exceeds > + BITS_PER_WORD? */ > + if (tbits > BITS_PER_WORD) > + break; > + > + /* Find the first available standard integral type. */ > + if (tbits >=3D min_ni_prec && targetm.scalar_mode_supported_p (tmode= )) > + { > + iv_type =3D build_nonstandard_integer_type (tbits, true); > + break; > + } > + } The outer {=E2=80=A6} block should be indented by two spaces relative to the FOR_EACH_MODE_IN_CLASS. > + > + if (!iv_type) > + { > + if (dump_enabled_p ()) > + dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, > + "can't vectorize with length-based partial vectors" > + " due to no suitable iv type.\n"); IMO reads better as s/due to/because there is/ > + /* Shouldn't go with length-based approach if fully masked. */ > + gcc_assert (!loop_lens || (loop_lens && !loop_masks)); The =E2=80=9Cloop_lens &&=E2=80=9D is redundant. Same for vectorizable_load. > @@ -7994,6 +8030,42 @@ vectorizable_store (vec_info *vinfo, > vect_finish_stmt_generation (vinfo, stmt_info, call, gsi); > new_stmt =3D call; > } > + else if (loop_lens) > + { > + tree final_len > + =3D vect_get_loop_len (loop_vinfo, loop_lens, > + vec_num * ncopies, vec_num * j + i); > + align =3D least_bit_hwi (misalign | align); > + tree ptr =3D build_int_cst (ref_type, align); > + machine_mode vmode =3D TYPE_MODE (vectype); > + opt_machine_mode new_ovmode > + =3D get_len_load_store_mode (vmode, false); > + gcc_assert (new_ovmode.exists ()); > + machine_mode new_vmode =3D new_ovmode.require (); The assert is redundant with the =E2=80=9Crequire ()=E2=80=9D. Same for vectorizable_load. Thanks, Richard