From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x22c.google.com (mail-lj1-x22c.google.com [IPv6:2a00:1450:4864:20::22c]) by sourceware.org (Postfix) with ESMTPS id 4248E3858D33 for ; Fri, 8 Mar 2024 14:03:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4248E3858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4248E3858D33 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::22c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709906591; cv=none; b=FOpq9WjAuOGBPwMw5wdPrxeusfnZK6vIG40vLbXj+04Czt7nI6jccrL/1ZwmeBCmWEISWm+iIuD4QyqrT+kbu29kW+ICsROZiwreWZEhEEXsdqV8SwCMklP2jSAC+JoPZnaP9aIUz9Fby6BbITsiksaqH7v3c3h7MfgY+2opk8k= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709906591; c=relaxed/simple; bh=4nFcMgYDj639k54NiaYZb+ozjWHs41S23tqKXK1GVT8=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=CJVq7IS4leQkdnaYMW5XOfMuf7tbXbYanT+dGnerFhe/eQ7e/mTlbfqS5rMCh0b4/wYgMs3FYtWKaApJSglIyKLWBynHxeP0QPVJyjpVjeq/DEzVWMgz2OkWQNxlu6KN5tz/WMTsJoBIrwGQ/+kBkDa4sNWl/FXv38vwWCgNqEw= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lj1-x22c.google.com with SMTP id 38308e7fff4ca-2d27184197cso26308051fa.1 for ; Fri, 08 Mar 2024 06:03:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1709906588; x=1710511388; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=D2wKRFTjWoppLBGd2Hij2YAP7IcjAh4KJd0k/wlzwXM=; b=FTAEvox8ml/BuXOC1QfdafRwdlbQBKMub+bX6yE6QnrOqUIVJKvrFc4j/4TPZmMfMl VbHl9qdA0oiUuQUa4GdefYl3HxyboNPFpk8GqOFEP47y7Z5u7WIf45KehNPG/IdhreQV Sieo57WeZ5S3m766DrvphAr4OuvQnWPPQC9MavlZM5AVDcmY0c5AwAHNPRBi/64A2vwg aZH6OZNBrOWSCeh/+Ae4UvB8GzLLCoDomSNRxbSAtIPZCwNpug6hmZkDiAoXmjVh3IpD Vn2SUGe1SQPRTqImFoBrafiHGy9/VbLOXeREpD2ztWb4HFl+meGoc2dnvvMQex8LhCPV UXyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709906588; x=1710511388; h=content-transfer-encoding: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=D2wKRFTjWoppLBGd2Hij2YAP7IcjAh4KJd0k/wlzwXM=; b=TwNxwyiGWvUTDMRIH3UlQFD7c6RZEeECX4k4CvZReHCHpPwAHUJ+6UDej8hUPDrgB7 yxHy46Nh2Zd+k7bcMWWs/mxcleI9zsGpMTVDM48LtDVNgYbztaFYRUM4Jky7x/2sPeD0 BXG0mI7R3mBDgkTYQEXsqYvu8YKCgUO3PMdgcK0gA9ibI9rx8eA1fITfp4zrHiBIYV/J oFCNklOgXpCLs9JOXfgGKjI+QSik9groV8CEQwT77in+GGuHe/rEVawPygWoCALl9Brb 0wlDKqrNVuaR7nFSSA/cwzDTSVVx4VHmWfnGbyhjlneyauAlOTBAEQJWJVanS/vyhzS7 GfDA== X-Gm-Message-State: AOJu0Ywxu2E+0zFylXOOFgTgdoDyZhe6emGKkYcT9Kp+Ozsn0n46ovqB EfYjboSDEAU1Mpiy1lT80l3X5IWPmLO2jnvohLw3gEdnvaD7TmUgQ3u1/02++BxGfroxmffJCzf WhNJGS14JpqYkx3zvmIAPoQPlqjo= X-Google-Smtp-Source: AGHT+IHnJajwOP/G9ojvkTCMvO3S4F/IOjslZwlNltZ0F/WhjmLgSghhb7um2ORoVwlRJlzY+/VnccVHPFsaSCVNfqs= X-Received: by 2002:a2e:3010:0:b0:2d3:fe9a:ecf9 with SMTP id w16-20020a2e3010000000b002d3fe9aecf9mr3346058ljw.7.1709906587334; Fri, 08 Mar 2024 06:03:07 -0800 (PST) MIME-Version: 1.0 References: <20240308000401.2766685-1-pan2.li@intel.com> In-Reply-To: From: Richard Biener Date: Fri, 8 Mar 2024 15:02:56 +0100 Message-ID: Subject: Re: [PATCH v1] VECT: Bugfix ICE for vectorizable_store when both len and mask To: pan2.li@intel.com Cc: gcc-patches@gcc.gnu.org, juzhe.zhong@rivai.ai, kito.cheng@gmail.com, yanzhang.wang@intel.com, rdapp.gcc@gmail.com, jeffreyalaw@gmail.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE,WEIRD_PORT 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 Fri, Mar 8, 2024 at 2:59=E2=80=AFPM Richard Biener wrote: > > On Fri, Mar 8, 2024 at 1:04=E2=80=AFAM wrote: > > > > From: Pan Li > > > > This patch would like to fix one ICE in vectorizable_store for both the > > loop_masks and loop_lens. The ICE looks like below with "-march=3Drv64= gcv -O3". > > > > during GIMPLE pass: vect > > test.c: In function =E2=80=98d=E2=80=99: > > test.c:6:6: internal compiler error: in vectorizable_store, at > > tree-vect-stmts.cc:8691 > > 6 | void d() { > > | ^ > > 0x37a6f2f vectorizable_store > > .../__RISC-V_BUILD__/../gcc/tree-vect-stmts.cc:8691 > > 0x37b861c vect_analyze_stmt(vec_info*, _stmt_vec_info*, bool*, > > _slp_tree*, _slp_instance*, vec*) > > .../__RISC-V_BUILD__/../gcc/tree-vect-stmts.cc:13242 > > 0x1db5dca vect_analyze_loop_operations > > .../__RISC-V_BUILD__/../gcc/tree-vect-loop.cc:2208 > > 0x1db885b vect_analyze_loop_2 > > .../__RISC-V_BUILD__/../gcc/tree-vect-loop.cc:3041 > > 0x1dba029 vect_analyze_loop_1 > > .../__RISC-V_BUILD__/../gcc/tree-vect-loop.cc:3481 > > 0x1dbabad vect_analyze_loop(loop*, vec_info_shared*) > > .../__RISC-V_BUILD__/../gcc/tree-vect-loop.cc:3639 > > 0x1e389d1 try_vectorize_loop_1 > > .../__RISC-V_BUILD__/../gcc/tree-vectorizer.cc:1066 > > 0x1e38f3d try_vectorize_loop > > .../__RISC-V_BUILD__/../gcc/tree-vectorizer.cc:1182 > > 0x1e39230 execute > > .../__RISC-V_BUILD__/../gcc/tree-vectorizer.cc:1298 > > > > Given the masks and the lens cannot be enabled simultanously when loop = is > > using partial vectors. Thus, we need to ensure the one is disabled whe= n we > > would like to record the other in check_load_store_for_partial_vectors.= For > > example, when we try to record loop len, we need to check if the loop m= ask > > is disabled or not. > > I don't think you can rely on LOOP_VINFO_FULLY_WITH_LENGTH_P during > analysis. Instead how we tried to set up things is that we never even tr= y > both and there is (was?) code to reject partial vector usage when we end > up recording both lens and masks. That is, a fix along what you do would have been to split LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P into _WITH_LENGTH and _WITH_MASKS, make sure to record both when a stmt can handle both so in the end we'll have a choice. Currently if we end up with both mask and len we don't know whether all stmts support lens or masks or just some. But we simply assumed on RISCV you'd never end up with unsupported len but supported mask I guess. Richard. > That said, the assert you run into should be only asserted during transfo= rm, > not during analysis. It possibly was before Robins costing reorg? > > Richard. > > > Below testsuites are passed for this patch: > > * The x86 bootstrap tests. > > * The x86 fully regression tests. > > * The aarch64 fully regression tests. > > * The riscv fully regressison tests. > > > > PR target/114195 > > > > gcc/ChangeLog: > > > > * tree-vect-stmts.cc (check_load_store_for_partial_vectors): Ad= d > > loop mask/len check before recording as they are mutual exclusi= on. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/riscv/rvv/base/pr114195-1.c: New test. > > > > Signed-off-by: Pan Li > > --- > > .../gcc.target/riscv/rvv/base/pr114195-1.c | 15 +++++++++++ > > gcc/tree-vect-stmts.cc | 26 ++++++++++++++----- > > 2 files changed, 35 insertions(+), 6 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114195-1.= c > > > > diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114195-1.c b/gcc= /testsuite/gcc.target/riscv/rvv/base/pr114195-1.c > > new file mode 100644 > > index 00000000000..b0c9d5b81b8 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114195-1.c > > @@ -0,0 +1,15 @@ > > +/* Test that we do not have ice when compile */ > > +/* { dg-do compile } */ > > +/* { dg-options "-march=3Drv64gcv -mabi=3Dlp64d -O3 -ftree-vectorize" = } */ > > + > > +long a, b; > > +extern short c[]; > > + > > +void d() { > > + for (int e =3D 0; e < 35; e +=3D 2) { > > + a =3D ({ a < 0 ? a : 0; }); > > + b =3D ({ b < 0 ? b : 0; }); > > + > > + c[e] =3D 0; > > + } > > +} > > diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc > > index 14a3ffb5f02..624947ed271 100644 > > --- a/gcc/tree-vect-stmts.cc > > +++ b/gcc/tree-vect-stmts.cc > > @@ -1502,6 +1502,8 @@ check_load_store_for_partial_vectors (loop_vec_in= fo loop_vinfo, tree vectype, > > gather_scatter_info *gs_info, > > tree scalar_mask) > > { > > + gcc_assert (LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo)); > > + > > /* Invariant loads need no special support. */ > > if (memory_access_type =3D=3D VMAT_INVARIANT) > > return; > > @@ -1521,9 +1523,17 @@ check_load_store_for_partial_vectors (loop_vec_i= nfo loop_vinfo, tree vectype, > > internal_fn ifn > > =3D (is_load ? vect_load_lanes_supported (vectype, group_size, = true) > > : vect_store_lanes_supported (vectype, group_size, t= rue)); > > - if (ifn =3D=3D IFN_MASK_LEN_LOAD_LANES || ifn =3D=3D IFN_MASK_LE= N_STORE_LANES) > > + > > + /* When the loop_vinfo using partial vector, we cannot enable b= oth > > + the fully mask and length simultaneously. Thus, make sure the > > + other one is disabled when record one of them. > > + The same as other place for both the vect_record_loop_len and > > + vect_record_loop_mask. */ > > + if ((ifn =3D=3D IFN_MASK_LEN_LOAD_LANES || ifn =3D=3D IFN_MASK_L= EN_STORE_LANES) > > + && !LOOP_VINFO_FULLY_MASKED_P (loop_vinfo)) > > vect_record_loop_len (loop_vinfo, lens, nvectors, vectype, 1); > > - else if (ifn =3D=3D IFN_MASK_LOAD_LANES || ifn =3D=3D IFN_MASK_S= TORE_LANES) > > + else if ((ifn =3D=3D IFN_MASK_LOAD_LANES || ifn =3D=3D IFN_MASK_= STORE_LANES) > > + && !LOOP_VINFO_FULLY_WITH_LENGTH_P (loop_vinfo)) > > vect_record_loop_mask (loop_vinfo, masks, nvectors, vectype, > > scalar_mask); > > else > > @@ -1549,12 +1559,14 @@ check_load_store_for_partial_vectors (loop_vec_= info loop_vinfo, tree vectype, > > if (internal_gather_scatter_fn_supported_p (len_ifn, vectype, > > gs_info->memory_type, > > gs_info->offset_vecty= pe, > > - gs_info->scale)) > > + gs_info->scale) > > + && !LOOP_VINFO_FULLY_MASKED_P (loop_vinfo)) > > vect_record_loop_len (loop_vinfo, lens, nvectors, vectype, 1); > > else if (internal_gather_scatter_fn_supported_p (ifn, vectype, > > gs_info->memory_= type, > > gs_info->offset_= vectype, > > - gs_info->scale)) > > + gs_info->scale) > > + && !LOOP_VINFO_FULLY_WITH_LENGTH_P (loop_vinfo)) > > vect_record_loop_mask (loop_vinfo, masks, nvectors, vectype, > > scalar_mask); > > else > > @@ -1608,7 +1620,8 @@ check_load_store_for_partial_vectors (loop_vec_in= fo loop_vinfo, tree vectype, > > machine_mode mask_mode; > > machine_mode vmode; > > bool using_partial_vectors_p =3D false; > > - if (get_len_load_store_mode (vecmode, is_load).exists (&vmode)) > > + if (get_len_load_store_mode (vecmode, is_load).exists (&vmode) > > + && !LOOP_VINFO_FULLY_MASKED_P (loop_vinfo)) > > { > > nvectors =3D group_memory_nvectors (group_size * vf, nunits); > > unsigned factor =3D (vecmode =3D=3D vmode) ? 1 : GET_MODE_UNIT_S= IZE (vecmode); > > @@ -1616,7 +1629,8 @@ check_load_store_for_partial_vectors (loop_vec_in= fo loop_vinfo, tree vectype, > > using_partial_vectors_p =3D true; > > } > > else if (targetm.vectorize.get_mask_mode (vecmode).exists (&mask_mod= e) > > - && can_vec_mask_load_store_p (vecmode, mask_mode, is_load)) > > + && can_vec_mask_load_store_p (vecmode, mask_mode, is_load) > > + && !LOOP_VINFO_FULLY_WITH_LENGTH_P (loop_vinfo)) > > { > > nvectors =3D group_memory_nvectors (group_size * vf, nunits); > > vect_record_loop_mask (loop_vinfo, masks, nvectors, vectype, sca= lar_mask); > > -- > > 2.34.1 > >