From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com [IPv6:2a00:1450:4864:20::22a]) by sourceware.org (Postfix) with ESMTPS id 186663858D33 for ; Thu, 2 Mar 2023 07:56:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 186663858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lj1-x22a.google.com with SMTP id z5so16688788ljc.8 for ; Wed, 01 Mar 2023 23:56:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1677743789; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=z4fUccQO5jCR10s5xIEDbBnmNLWoegdU8WptgyWWMLw=; b=LuW3R0gkioLnP8YW84537itZzeTL3fTor2PxEKAFUu8r1UbS7GPoS7AMsgsa8c/BMt iQrpTiCqRk1C9TDY2FkYLFB280YFTzUrdKMUvR7CbZHzF0mns+6kxmYVjdyU8C/qYwfH DamFQuVQ9mzPVJ4qLJNHOs7f+Gq7Cy7YVNxjUfKPwo7v75Xe+FHOnS4EJhT/c+lSvbuD oh3ibxYKlJGpMghrzDWf1Nmowoc9jDs4jAgDa1s0xCEeL+4KAls9g7L2eHSH5a/BBjlv VQ0+3bgdFWDVdF6ftBty2i7zbJZPR/IpqVJkUdUj7pnqn6IKYZc1wtrvR3+v+5ewQFzR ccIQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677743789; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=z4fUccQO5jCR10s5xIEDbBnmNLWoegdU8WptgyWWMLw=; b=F8qzHVv1x1WHUhVg9ZS6z4kTxthhZPuegAxlO3WcFmx6B9C0YnK8juO2TDboSy8rJW EblmYG/uul64WQh9+buxrh134C+rzfmvLEeMaYzXy/m4r9k8LE4qFZDXe+wcCFRdfyQp kDsm1XDZvEJaEjk0USjJJY6EZ3AewmMG3LnpV4WunWuF0e/M3GXr7Vc5pbpmgwUu17gr Z7n6GoT++Og4rAYX5orGkWK28IV07zDnZM+WfmyT+Oi3g9iA5qn+LnAofzDTbvJaOCbF 2sFEe86Ev1c+585i1WR3LiYaTyte+huSrP7hcV0JtBi7d2xU86Hx9yhHCrffWcKoxfp+ AvFw== X-Gm-Message-State: AO0yUKUttDu5PkY2vdCFtFjzymO5LFRCVG/f98k6YSidxTWwy9m1j8aS 9LJrc+Y7RZpr71KsUesle7fVABYZ9i4/zseDPlM= X-Google-Smtp-Source: AK7set8P1lCVKQ6QfS+hcV6plKsVRAQklLmvGT3STcgNWE+s/gzC616COJSLFcbYHVmlcuDyt8wZOtI1XXQTuIGhF0Y= X-Received: by 2002:a05:651c:1243:b0:295:b77c:a3a2 with SMTP id h3-20020a05651c124300b00295b77ca3a2mr2868337ljh.6.1677743789413; Wed, 01 Mar 2023 23:56:29 -0800 (PST) MIME-Version: 1.0 References: <3356a2e0-b402-de07-9374-6e5b5c59a2f2@rivosinc.com> In-Reply-To: From: Richard Biener Date: Thu, 2 Mar 2023 08:56:17 +0100 Message-ID: Subject: Re: [PATCH] vect: Check that vector factor is a compile-time constant To: Michael Collison Cc: gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-7.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP 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, Mar 1, 2023 at 10:00 PM Michael Collison wrote: > > Okay there seems to be consensus on using constant_lower_bound (vf), but > I don't understand how that is a replacement for "vf.is_constant ()"? In > one case we are checking if "vf" is a constant, on the other we are > asking for the lower bound. For the crash in question > "constant_lower_bound (vf) " returns the integer value of two. Use the result of constant_lower_bound (vf) in place of 'vf' when build_int_cst. That should be correct for both poly and non-poly int VF. > On 2/27/23 09:51, Richard Sandiford wrote: > > FWIW, this patch looks good to me. I'd argue it's a regression fix > > of kinds, in that the current code was correct before variable VF and > > became incorrect after variable VF. It might be possible to trigger > > the problem on SVE too, with a sufficiently convoluted test case. > > (Haven't tried though.) > > > > Richard Biener writes: > >> On Wed, Feb 22, 2023 at 12:03 AM Michael Collison wrote: > >>> While working on autovectorizing for the RISCV port I encountered an > >>> issue where vect_do_peeling assumes that the vectorization factor is a > >>> compile-time constant. The vectorization is not a compile-time constant > >>> on RISCV. > >>> > >>> Tested on RISCV and x86_64-linux-gnu. Okay? > >> I wonder how you arrive at prologue peeling with a non-constant VF? > > Not sure about the RVV case, but I think it makes sense in principle. > > E.g. if some ISA takes the LOAD_LEN rather than fully-predicated > > approach, it can't easily use the first iteration of the vector loop > > to do peeling for alignment. (At least, the IV steps would then > > no longer match VF for all iterations.) I guess it could use a > > *different* vector loop, but we don't support that yet. > > > > There are also some corner cases for which we still don't support > > predicated loops and instead fall back on an unpredicated VLA loop > > followed by a scalar epilogue. Peeling for alignment would then > > require a scalar prologue too. > > > >> In any case it would probably be better to use constant_lower_bound (vf) > >> here? Also it looks wrong to apply this limit in case we are using > >> a fully masked main vector loop. But as said, the specific case of > >> non-constant VF and prologue peeling probably wasn't supposed to happen, > >> instead the prologue usually is applied via an offset to a fully masked loop? > > Hmm, yeah, agree constant_lower_bound should work too. > > > > Thanks, > > Richard > > > >> Richard? > >> > >> Thanks, > >> Richard. > >> > >>> Michael > >>> > >>> gcc/ > >>> > >>> * tree-vect-loop-manip.cc (vect_do_peeling): Verify > >>> that vectorization factor is a compile-time constant. > >>> > >>> --- > >>> gcc/tree-vect-loop-manip.cc | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc > >>> index 6aa3d2ed0bf..1ad1961c788 100644 > >>> --- a/gcc/tree-vect-loop-manip.cc > >>> +++ b/gcc/tree-vect-loop-manip.cc > >>> @@ -2930,7 +2930,7 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree > >>> niters, tree nitersm1, > >>> niters = vect_build_loop_niters (loop_vinfo, &new_var_p); > >>> /* It's guaranteed that vector loop bound before vectorization is at > >>> least VF, so set range information for newly generated var. */ > >>> - if (new_var_p) > >>> + if (new_var_p && vf.is_constant ()) > >>> { > >>> value_range vr (type, > >>> wi::to_wide (build_int_cst (type, vf)), > >>> -- > >>> 2.34.1 > >>>