From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x630.google.com (mail-pl1-x630.google.com [IPv6:2607:f8b0:4864:20::630]) by sourceware.org (Postfix) with ESMTPS id A44333858439 for ; Wed, 24 May 2023 02:10:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A44333858439 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=sifive.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=sifive.com Received: by mail-pl1-x630.google.com with SMTP id d9443c01a7336-1ae557aaf1dso3609775ad.2 for ; Tue, 23 May 2023 19:10:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; t=1684894218; x=1687486218; 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=rpJK0L7DqM2KsnY8V3nY55XHZ9jJSpU8i3YiabZr4TQ=; b=aH8TIB8i+ZC3GG5yCDGmF6VYml3o4BMJ95dH6Em0yRLjYgwZIpDluH8DZutKN89JtW 8C/s93yAZ8VU8AI5NKTRPiHRTdHbmA/CYY39KxGMD7DdmeRcc2QdqLtmZZCpekW+JCwZ KaKZrKKymhZmDBlDEcUzSo28dqu7CsEsX8/akIoJQZlrIMjXKZgf0kZLGX8SMN6HfWeH VE8Y7mC0YAJDrP+8uuNftUIpIsS3zk2rHSTvYNK7PrTumyB7lmRPlQZoys1eIPvHa+nU RUWpppIXH5Q3NPSMxl7Nl5lfimjjgguzzalqrz1DMYJoy7wRaFSWpS3uv6JdiCiHO4FC tcTQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684894218; x=1687486218; 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=rpJK0L7DqM2KsnY8V3nY55XHZ9jJSpU8i3YiabZr4TQ=; b=iKfcqI22u714rE/dAV7l3Q5o4j/HIW8BM0OXJeAxcasIwJ9WEMR7JB4trGERD3auRv ylsKQUW6IgfoFYf3Nry7Z7gxDpYR/tYXSX51trNxeT9+U0CHWEUkQn034vm1IvreqHQK r+M3qBZ2CF+3lTyMgToQdZqdIBJd7kV5lkkt1pevMql3o3+T8e9HGk6+6Z04q0gmLrnc c1HKJswlEcsJGUinqOX5DSz4YNaz+rk9ehg2fTxdsYoHIWwqPgB9OnmDtiNAKm3GJlTn ZaXGvs2E3gIzcwp4nV3QFRq+G8IkuN67Lww7G2s0hhgxj4/fBC0NbutX+h9KDKV0ow/I yomQ== X-Gm-Message-State: AC+VfDzsvtbsfK4/cMStNK435N1OD5Hbmt3Gydp1Xdz30WUvZXVBuI1l 4PxcydHwi51b1CZsAhs54p9XMkVo5a4mks2lwpVPTA== X-Google-Smtp-Source: ACHHUZ5FccNJusI4OV3aiTAWzdW+20BHPU8njh/YoEPacV8sLIdC8QVVprmERUq477fEL4Ogfe3j6dFISKQ5aDPFxLY= X-Received: by 2002:a17:903:183:b0:1af:bb31:7d3 with SMTP id z3-20020a170903018300b001afbb3107d3mr8501667plg.51.1684894218461; Tue, 23 May 2023 19:10:18 -0700 (PDT) MIME-Version: 1.0 References: <20230524015727.1157568-1-juzhe.zhong@rivai.ai> In-Reply-To: <20230524015727.1157568-1-juzhe.zhong@rivai.ai> From: Kito Cheng Date: Wed, 24 May 2023 10:10:07 +0800 Message-ID: Subject: Re: [PATCH] RISC-V: Fix incorrect code of touching inaccessible memory address To: juzhe.zhong@rivai.ai Cc: gcc-patches@gcc.gnu.org, kito.cheng@gmail.com, palmer@dabbelt.com, palmer@rivosinc.com, jeffreyalaw@gmail.com, rdapp.gcc@gmail.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,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: I am a little hesitant about that, since I feel the vl and normal op should be put in separately, otherwise the means of m_op_num is kind of unclear, we have comments there but I think it's not ideal since it is really context sensitive and hard to determine. And I suspect gcc_assert (ops[m_op_num]); is not too useful since it might just be out of range access if we forgot to pass the vl operands. I am thinking we might need to introduce something like llvm::ArrayRef to have a better sanity check, e.g. check the length of ops. One possible solution is just using std::vector can achieve the same purpose too, but come with more cost. On Wed, May 24, 2023 at 9:57=E2=80=AFAM wrote: > > From: Juzhe-Zhong > > For VLMAX situation, rtx len =3D ops[m_op_num] is incorrect since > the last element the ops array should be ops[m_op_num - 1]; > > I notice this issue when I am debugging code. > This is a code bug even though the following codes will hide this issue. > We still should need this minor fix. > > Built && Regression PASSed. > > Ok for trunk? > > gcc/ChangeLog: > > * config/riscv/riscv-v.cc: Fix bug of touching inaccessible memor= y. > > --- > gcc/config/riscv/riscv-v.cc | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc > index fa61a850a22..a0992773644 100644 > --- a/gcc/config/riscv/riscv-v.cc > +++ b/gcc/config/riscv/riscv-v.cc > @@ -169,7 +169,11 @@ public: > > if (m_needs_avl_p) > { > - rtx len =3D ops[m_op_num]; > + /* The variable "m_op_num" means the real operation operands exce= pt VL > + operand. For VLMAX patterns (no VL operand), the last operand = is > + ops[m_op_num -1]. Wheras for non-VLMAX patterns, the last oper= and is > + VL operand which is ops[m_op_num]. */ > + rtx len =3D NULL_RTX; > if (m_vlmax_p) > { > if (const_vlmax_p (m_dest_mode)) > @@ -185,6 +189,20 @@ public: > len =3D gen_reg_rtx (Pmode); > emit_vlmax_vsetvl (m_dest_mode, len); > } > + else > + { > + /* According to LRA mov pattern in vector.md. The VL oper= and is > + always the last operand. */ > + gcc_assert (ops[m_op_num]); > + len =3D ops[m_op_num]; > + } > + } > + else > + { > + /* For non-VLMAX patterns. The VL operand is always the last > + * operand. */ > + gcc_assert (ops[m_op_num]); > + len =3D ops[m_op_num]; > } > add_input_operand (len, Pmode); > } > -- > 2.36.3 >