Thanks. I fix it by separating VL and normal operand. V2 patch: https://gcc.gnu.org/pipermail/gcc-patches/2023-May/619356.html Does it look more reasonable to you? Just finished the building test && regression. Thanks. juzhe.zhong@rivai.ai From: Kito Cheng Date: 2023-05-24 10:10 To: juzhe.zhong CC: gcc-patches; kito.cheng; palmer; palmer; jeffreyalaw; rdapp.gcc Subject: Re: [PATCH] RISC-V: Fix incorrect code of touching inaccessible memory address 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 AM wrote: > > From: Juzhe-Zhong > > For VLMAX situation, rtx len = 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 memory. > > --- > 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 = ops[m_op_num]; > + /* The variable "m_op_num" means the real operation operands except VL > + operand. For VLMAX patterns (no VL operand), the last operand is > + ops[m_op_num -1]. Wheras for non-VLMAX patterns, the last operand is > + VL operand which is ops[m_op_num]. */ > + rtx len = NULL_RTX; > if (m_vlmax_p) > { > if (const_vlmax_p (m_dest_mode)) > @@ -185,6 +189,20 @@ public: > len = gen_reg_rtx (Pmode); > emit_vlmax_vsetvl (m_dest_mode, len); > } > + else > + { > + /* According to LRA mov pattern in vector.md. The VL operand is > + always the last operand. */ > + gcc_assert (ops[m_op_num]); > + len = ops[m_op_num]; > + } > + } > + else > + { > + /* For non-VLMAX patterns. The VL operand is always the last > + * operand. */ > + gcc_assert (ops[m_op_num]); > + len = ops[m_op_num]; > } > add_input_operand (len, Pmode); > } > -- > 2.36.3 >