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 9D9943858D37 for ; Thu, 20 Apr 2023 15:20:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9D9943858D37 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 274B21480; Thu, 20 Apr 2023 08:21:34 -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 9F9CC3F6C4; Thu, 20 Apr 2023 08:20:49 -0700 (PDT) From: Richard Sandiford To: "Andre Vieira \(lists\)" Mail-Followup-To: "Andre Vieira \(lists\)" ,"gcc-patches\@gcc.gnu.org" , "jakub\@redhat.com" , Richard Biener , richard.sandiford@arm.com Cc: "gcc-patches\@gcc.gnu.org" , "jakub\@redhat.com" , Richard Biener Subject: Re: [PATCH 1/X] omp: Replace simd_clone_subparts with TYPE_VECTOR_SUBPARTS References: <98e8127c-ecb0-2977-3c6c-29151edfcb15@arm.com> Date: Thu, 20 Apr 2023 16:20:48 +0100 In-Reply-To: <98e8127c-ecb0-2977-3c6c-29151edfcb15@arm.com> (Andre Vieira's message of "Wed, 8 Mar 2023 16:20:02 +0000") 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=-30.8 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,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: "Andre Vieira (lists)" writes: > Hi, > > This patch replaces the uses of simd_clone_subparts with > TYPE_VECTOR_SUBPARTS and removes the definition of the first. > > gcc/ChangeLog: > > * omp-sind-clone.cc (simd_clone_subparts): Remove. > (simd_clone_init_simd_arrays): Replace simd_clone_subparts with > TYPE_VECTOR_SUBPARTS. > (ipa_simd_modify_function_body): Likewise. > * tree-vect-stmts.cc (simd_clone_subparts): Remove. > (vectorizable_simd_clone_call): Replace simd_clone_subparts > with TYPE_VECTOR_SUBPARTS. > > diff --git a/gcc/omp-simd-clone.cc b/gcc/omp-simd-clone.cc > index 0949b8ba288dfc7e7692403bfc600983faddf5dd..48b480e7556d9ad8e5502e10e513ec36b17b9cbb 100644 > --- a/gcc/omp-simd-clone.cc > +++ b/gcc/omp-simd-clone.cc > @@ -255,16 +255,6 @@ ok_for_auto_simd_clone (struct cgraph_node *node) > return true; > } > > - > -/* Return the number of elements in vector type VECTYPE, which is associated > - with a SIMD clone. At present these always have a constant length. */ > - > -static unsigned HOST_WIDE_INT > -simd_clone_subparts (tree vectype) > -{ > - return TYPE_VECTOR_SUBPARTS (vectype).to_constant (); > -} > - > /* Allocate a fresh `simd_clone' and return it. NARGS is the number > of arguments to reserve space for. */ > > @@ -1027,7 +1017,7 @@ simd_clone_init_simd_arrays (struct cgraph_node *node, > } > continue; > } > - if (known_eq (simd_clone_subparts (TREE_TYPE (arg)), > + if (known_eq (TYPE_VECTOR_SUBPARTS (TREE_TYPE (arg)), > node->simdclone->simdlen)) > { > tree ptype = build_pointer_type (TREE_TYPE (TREE_TYPE (array))); > @@ -1039,7 +1029,7 @@ simd_clone_init_simd_arrays (struct cgraph_node *node, > } > else > { > - unsigned int simdlen = simd_clone_subparts (TREE_TYPE (arg)); > + poly_uint64 simdlen = TYPE_VECTOR_SUBPARTS (TREE_TYPE (arg)); > unsigned int times = vector_unroll_factor (node->simdclone->simdlen, > simdlen); > tree ptype = build_pointer_type (TREE_TYPE (TREE_TYPE (array))); > @@ -1225,9 +1215,9 @@ ipa_simd_modify_function_body (struct cgraph_node *node, > iter, NULL_TREE, NULL_TREE); > adjustments->register_replacement (&(*adjustments->m_adj_params)[j], r); > > - if (multiple_p (node->simdclone->simdlen, simd_clone_subparts (vectype))) > + if (multiple_p (node->simdclone->simdlen, TYPE_VECTOR_SUBPARTS (vectype))) > j += vector_unroll_factor (node->simdclone->simdlen, > - simd_clone_subparts (vectype)) - 1; > + TYPE_VECTOR_SUBPARTS (vectype)) - 1; > } > adjustments->sort_replacements (); > > diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc > index df6239a1c61c7213ad3c1468723bc1adf70bc02c..c85b6babc4bc5bc3111ef326dcc8f32bb25333f6 100644 > --- a/gcc/tree-vect-stmts.cc > +++ b/gcc/tree-vect-stmts.cc > @@ -3964,16 +3964,6 @@ vect_simd_lane_linear (tree op, class loop *loop, > } > } > > -/* Return the number of elements in vector type VECTYPE, which is associated > - with a SIMD clone. At present these vectors always have a constant > - length. */ > - > -static unsigned HOST_WIDE_INT > -simd_clone_subparts (tree vectype) > -{ > - return TYPE_VECTOR_SUBPARTS (vectype).to_constant (); > -} > - > /* Function vectorizable_simd_clone_call. > > Check if STMT_INFO performs a function call that can be vectorized > @@ -4251,7 +4241,7 @@ vectorizable_simd_clone_call (vec_info *vinfo, stmt_vec_info stmt_info, > slp_node); > if (arginfo[i].vectype == NULL > || !constant_multiple_p (bestn->simdclone->simdlen, > - simd_clone_subparts (arginfo[i].vectype))) > + TYPE_VECTOR_SUBPARTS (arginfo[i].vectype))) > return false; > } > > @@ -4349,15 +4339,19 @@ vectorizable_simd_clone_call (vec_info *vinfo, stmt_vec_info stmt_info, > case SIMD_CLONE_ARG_TYPE_VECTOR: > atype = bestn->simdclone->args[i].vector_type; > o = vector_unroll_factor (nunits, > - simd_clone_subparts (atype)); > + TYPE_VECTOR_SUBPARTS (atype)); > for (m = j * o; m < (j + 1) * o; m++) > { > - if (simd_clone_subparts (atype) > - < simd_clone_subparts (arginfo[i].vectype)) > + poly_uint64 atype_subparts = TYPE_VECTOR_SUBPARTS (atype); > + poly_uint64 arginfo_subparts > + = TYPE_VECTOR_SUBPARTS (arginfo[i].vectype); > + if (known_lt (atype_subparts, arginfo_subparts)) > { > poly_uint64 prec = GET_MODE_BITSIZE (TYPE_MODE (atype)); > - k = (simd_clone_subparts (arginfo[i].vectype) > - / simd_clone_subparts (atype)); > + if (!constant_multiple_p (atype_subparts, > + arginfo_subparts, &k)) > + gcc_unreachable (); > + Very minor, but I think it's conceptually cleaner to use the constant_multiple_p as the if condition, rather than known_lt. Then... > gcc_assert ((k & (k - 1)) == 0); > if (m == 0) > { > @@ -4387,8 +4381,9 @@ vectorizable_simd_clone_call (vec_info *vinfo, stmt_vec_info stmt_info, > } > else > { > - k = (simd_clone_subparts (atype) > - / simd_clone_subparts (arginfo[i].vectype)); > + if (!constant_multiple_p (arginfo_subparts, > + atype_subparts, &k)) > + gcc_unreachable (); ...make this else conditional on constant_multiple_p too, with a new final else that contains gcc_unreachable. > gcc_assert ((k & (k - 1)) == 0); > vec *ctor_elts; > if (k != 1) > @@ -4522,7 +4517,7 @@ vectorizable_simd_clone_call (vec_info *vinfo, stmt_vec_info stmt_info, > if (vec_dest) > { > gcc_assert (ratype > - || known_eq (simd_clone_subparts (rtype), nunits)); > + || known_eq (TYPE_VECTOR_SUBPARTS (rtype), nunits)); > if (ratype) > new_temp = create_tmp_var (ratype); > else if (useless_type_conversion_p (vectype, rtype)) > @@ -4536,13 +4531,13 @@ vectorizable_simd_clone_call (vec_info *vinfo, stmt_vec_info stmt_info, > > if (vec_dest) > { > - if (!multiple_p (simd_clone_subparts (vectype), nunits)) > + if (!multiple_p (TYPE_VECTOR_SUBPARTS (vectype), nunits)) > { > unsigned int k, l; > poly_uint64 prec = GET_MODE_BITSIZE (TYPE_MODE (vectype)); > poly_uint64 bytes = GET_MODE_SIZE (TYPE_MODE (vectype)); > k = vector_unroll_factor (nunits, > - simd_clone_subparts (vectype)); > + TYPE_VECTOR_SUBPARTS (vectype)); > gcc_assert ((k & (k - 1)) == 0); > for (l = 0; l < k; l++) > { > @@ -4568,10 +4563,12 @@ vectorizable_simd_clone_call (vec_info *vinfo, stmt_vec_info stmt_info, > vect_clobber_variable (vinfo, stmt_info, gsi, new_temp); > continue; > } > - else if (!multiple_p (nunits, simd_clone_subparts (vectype))) > + else if (!multiple_p (nunits, TYPE_VECTOR_SUBPARTS (vectype))) > { > - unsigned int k = (simd_clone_subparts (vectype) > - / simd_clone_subparts (rtype)); > + unsigned int k; > + if (!constant_multiple_p (TYPE_VECTOR_SUBPARTS (vectype), > + TYPE_VECTOR_SUBPARTS (rtype), &k)) > + gcc_unreachable (); Suggest using vector_unroll_factor here too. Thanks, Richard > gcc_assert ((k & (k - 1)) == 0); > if ((j & (k - 1)) == 0) > vec_alloc (ret_ctor_elts, k); > @@ -4579,7 +4576,7 @@ vectorizable_simd_clone_call (vec_info *vinfo, stmt_vec_info stmt_info, > { > unsigned int m, o; > o = vector_unroll_factor (nunits, > - simd_clone_subparts (rtype)); > + TYPE_VECTOR_SUBPARTS (rtype)); > for (m = 0; m < o; m++) > { > tree tem = build4 (ARRAY_REF, rtype, new_temp,