From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x536.google.com (mail-pg1-x536.google.com [IPv6:2607:f8b0:4864:20::536]) by sourceware.org (Postfix) with ESMTPS id D7AB83858D35 for ; Wed, 24 May 2023 01:32:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D7AB83858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com Received: by mail-pg1-x536.google.com with SMTP id 41be03b00d2f7-5346d150972so308095a12.3 for ; Tue, 23 May 2023 18:32:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20221208.gappssmtp.com; s=20221208; t=1684891962; x=1687483962; h=content-transfer-encoding:mime-version:message-id:to:from:cc :in-reply-to:subject:date:from:to:cc:subject:date:message-id :reply-to; bh=MIQWtTqW9fG5ZOh3RHk+/Jj+cEf1w4cLHBdyy15IU4w=; b=xo2Z4vDIWIrb3Z7rTpFg2t+4U0a2sXFFo5a942aG/Jeyqp4EUXCnz13PWtCwOTVKdB pt/2wO1n0Bu7aPS7bnNiEixKJLWiZncW/uwM0xyiLCZDn0B+dGKUXqwhWgZAcIZTd2h2 UQEHqHJKAAEuROo1YXyqZnO+NVC9CrTBt+YmvFVm8uhZ/bXmrL74fg7rlTaiP6hZws0j A3WlxaJnIx4Ya03NI9vsuFf2bBLg0oWRVp6ONwF0GGJLpjuheVzCikePSuVywWWgwVTd WuAOFH7YOJHl0qGWVjmYAgmuAbNKaeHcZFh/TTDvAtZe8pXT6VH7A4ZtEyHGXKEpr3u7 Lk4Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684891962; x=1687483962; h=content-transfer-encoding:mime-version:message-id:to:from:cc :in-reply-to:subject:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=MIQWtTqW9fG5ZOh3RHk+/Jj+cEf1w4cLHBdyy15IU4w=; b=M6oLq9rF7HnEC3DfSyLA75hT5R2Fz61Jx0BAKq4A+59zCfPJ2kzmynNqtgjkSdZ6pK NagajASmh92Y2iIalD5VA5KfypLOWojGfCTZkQBOCZT6LiCoFUPd+eyK3/IMxgyV1RVR h2t3dEbpL08x798fZFDCB6W+9p/0irv3uS0THdNxQsRUbHdvqNoSP6xPofhYLApUdveQ GXaoU04Z5UZ5FKejUvQz2RG1rS/eb3AMnV1lXrQ9yw16ucsoGn7txmE8EWBq/g3pxK0Z 0pMEttDChmkw1TvAtncEkUF1C5aZxWDinFeD2rnn0xEycRmahaQ26tQi/FgTovfmllh3 gZaQ== X-Gm-Message-State: AC+VfDx3t55wIHCxMeukJiVsbNDjd7Cv/ySuk4XG9f+0NoJANOlmGm5r aXU9+0AdV2Ip6hOvgwdQl5JWRFF9SbY3/jqo6vU= X-Google-Smtp-Source: ACHHUZ6GLOiE95nodaEW0Bp7wwx+Bz95CHKWS3G1GwASjEL/TtG8erNM3uzOAYXGTfLfT7NUNccBbA== X-Received: by 2002:a05:6a20:c78f:b0:102:18a9:cad7 with SMTP id hk15-20020a056a20c78f00b0010218a9cad7mr14879723pzb.58.1684891962305; Tue, 23 May 2023 18:32:42 -0700 (PDT) Received: from localhost ([50.221.140.188]) by smtp.gmail.com with ESMTPSA id o3-20020a655bc3000000b0051303d3e3c5sm5919541pgr.42.2023.05.23.18.32.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 May 2023 18:32:41 -0700 (PDT) Date: Tue, 23 May 2023 18:32:41 -0700 (PDT) X-Google-Original-Date: Tue, 23 May 2023 18:32:13 PDT (-0700) Subject: Re: [PATCH V2] RISC-V: Fix magic number of RVV auto-vectorization expander In-Reply-To: <20230524012848.1097889-1-juzhe.zhong@rivai.ai> CC: gcc-patches@gcc.gnu.org, Kito Cheng , kito.cheng@sifive.com, jeffreyalaw@gmail.com, rdapp.gcc@gmail.com, juzhe.zhong@rivai.ai From: Palmer Dabbelt To: juzhe.zhong@rivai.ai Message-ID: Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE,URIBL_BLACK 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 Tue, 23 May 2023 18:28:48 PDT (-0700), juzhe.zhong@rivai.ai wrote: > From: Juzhe-Zhong > > This simple patch fixes the magic number, remove magic number make codes more reasonable. > > Ok for trunk ? > > gcc/ChangeLog: > > * config/riscv/riscv-v.cc (expand_vec_series): Remove magic number. > (expand_const_vector): Ditto. > (legitimize_move): Ditto. > (sew64_scalar_helper): Ditto. > (expand_tuple_move): Ditto. > (expand_vector_init_insert_elems): Ditto. > * config/riscv/riscv.cc (vector_zero_call_used_regs): Ditto. > > --- > gcc/config/riscv/riscv-v.cc | 53 +++++++++++++++++-------------------- > gcc/config/riscv/riscv.cc | 2 +- > 2 files changed, 26 insertions(+), 29 deletions(-) > > diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc > index 478a052a779..fa61a850a22 100644 > --- a/gcc/config/riscv/riscv-v.cc > +++ b/gcc/config/riscv/riscv-v.cc > @@ -406,14 +406,14 @@ expand_vec_series (rtx dest, rtx base, rtx step) > int shift = exact_log2 (INTVAL (step)); > rtx shift_amount = gen_int_mode (shift, Pmode); > insn_code icode = code_for_pred_scalar (ASHIFT, mode); > - rtx ops[3] = {step_adj, vid, shift_amount}; > - emit_vlmax_insn (icode, riscv_vector::RVV_BINOP, ops); > + rtx ops[] = {step_adj, vid, shift_amount}; > + emit_vlmax_insn (icode, RVV_BINOP, ops); Looks like it also removes the "riscv_vector" namespace from some of the constants? No big deal, it's just a different cleanup (assuming it still builds and such). > } > else > { > insn_code icode = code_for_pred_scalar (MULT, mode); > - rtx ops[3] = {step_adj, vid, step}; > - emit_vlmax_insn (icode, riscv_vector::RVV_BINOP, ops); > + rtx ops[] = {step_adj, vid, step}; > + emit_vlmax_insn (icode, RVV_BINOP, ops); > } > } > > @@ -428,8 +428,8 @@ expand_vec_series (rtx dest, rtx base, rtx step) > { > rtx result = gen_reg_rtx (mode); > insn_code icode = code_for_pred_scalar (PLUS, mode); > - rtx ops[3] = {result, step_adj, base}; > - emit_vlmax_insn (icode, riscv_vector::RVV_BINOP, ops); > + rtx ops[] = {result, step_adj, base}; > + emit_vlmax_insn (icode, RVV_BINOP, ops); > emit_move_insn (dest, result); > } > } > @@ -445,8 +445,8 @@ expand_const_vector (rtx target, rtx src) > gcc_assert ( > const_vec_duplicate_p (src, &elt) > && (rtx_equal_p (elt, const0_rtx) || rtx_equal_p (elt, const1_rtx))); > - rtx ops[2] = {target, src}; > - emit_vlmax_insn (code_for_pred_mov (mode), riscv_vector::RVV_UNOP, ops); > + rtx ops[] = {target, src}; > + emit_vlmax_insn (code_for_pred_mov (mode), RVV_UNOP, ops); > return; > } > > @@ -458,16 +458,14 @@ expand_const_vector (rtx target, rtx src) > we use vmv.v.i instruction. */ > if (satisfies_constraint_vi (src) || satisfies_constraint_Wc0 (src)) > { > - rtx ops[2] = {tmp, src}; > - emit_vlmax_insn (code_for_pred_mov (mode), riscv_vector::RVV_UNOP, > - ops); > + rtx ops[] = {tmp, src}; > + emit_vlmax_insn (code_for_pred_mov (mode), RVV_UNOP, ops); > } > else > { > elt = force_reg (elt_mode, elt); > - rtx ops[2] = {tmp, elt}; > - emit_vlmax_insn (code_for_pred_broadcast (mode), > - riscv_vector::RVV_UNOP, ops); > + rtx ops[] = {tmp, elt}; > + emit_vlmax_insn (code_for_pred_broadcast (mode), RVV_UNOP, ops); > } > > if (tmp != target) > @@ -536,9 +534,8 @@ legitimize_move (rtx dest, rtx src) > rtx tmp = gen_reg_rtx (mode); > if (MEM_P (src)) > { > - rtx ops[2] = {tmp, src}; > - emit_vlmax_insn (code_for_pred_mov (mode), riscv_vector::RVV_UNOP, > - ops); > + rtx ops[] = {tmp, src}; > + emit_vlmax_insn (code_for_pred_mov (mode), RVV_UNOP, ops); > } > else > emit_move_insn (tmp, src); > @@ -548,8 +545,8 @@ legitimize_move (rtx dest, rtx src) > if (satisfies_constraint_vu (src)) > return false; > > - rtx ops[2] = {dest, src}; > - emit_vlmax_insn (code_for_pred_mov (mode), riscv_vector::RVV_UNOP, ops); > + rtx ops[] = {dest, src}; > + emit_vlmax_insn (code_for_pred_mov (mode), RVV_UNOP, ops); > return true; > } > > @@ -813,7 +810,7 @@ sew64_scalar_helper (rtx *operands, rtx *scalar_op, rtx vl, > *scalar_op = force_reg (scalar_mode, *scalar_op); > > rtx tmp = gen_reg_rtx (vector_mode); > - rtx ops[3] = {tmp, *scalar_op, vl}; > + rtx ops[] = {tmp, *scalar_op, vl}; > riscv_vector::emit_nonvlmax_insn (code_for_pred_broadcast (vector_mode), > riscv_vector::RVV_UNOP, ops); > emit_vector_func (operands, tmp); > @@ -1122,9 +1119,9 @@ expand_tuple_move (rtx *ops) > > if (fractional_p) > { > - rtx operands[3] = {subreg, mem, ops[4]}; > - emit_vlmax_insn (code_for_pred_mov (subpart_mode), > - riscv_vector::RVV_UNOP, operands); > + rtx operands[] = {subreg, mem, ops[4]}; > + emit_vlmax_insn (code_for_pred_mov (subpart_mode), RVV_UNOP, > + operands); > } > else > emit_move_insn (subreg, mem); > @@ -1147,9 +1144,9 @@ expand_tuple_move (rtx *ops) > > if (fractional_p) > { > - rtx operands[3] = {mem, subreg, ops[4]}; > - emit_vlmax_insn (code_for_pred_mov (subpart_mode), > - riscv_vector::RVV_UNOP, operands); > + rtx operands[] = {mem, subreg, ops[4]}; > + emit_vlmax_insn (code_for_pred_mov (subpart_mode), RVV_UNOP, > + operands); > } > else > emit_move_insn (mem, subreg); > @@ -1281,8 +1278,8 @@ expand_vector_init_insert_elems (rtx target, const rvv_builder &builder, > unsigned int unspec > = FLOAT_MODE_P (mode) ? UNSPEC_VFSLIDE1DOWN : UNSPEC_VSLIDE1DOWN; > insn_code icode = code_for_pred_slide (unspec, mode); > - rtx ops[3] = {target, target, builder.elt (i)}; > - emit_vlmax_insn (icode, riscv_vector::RVV_BINOP, ops); > + rtx ops[] = {target, target, builder.elt (i)}; > + emit_vlmax_insn (icode, RVV_BINOP, ops); > } > } > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > index e7300b2e97c..09fc9e5d95e 100644 > --- a/gcc/config/riscv/riscv.cc > +++ b/gcc/config/riscv/riscv.cc > @@ -7396,7 +7396,7 @@ vector_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs) > emitted_vlmax_vsetvl = true; > } > > - rtx ops[3] = {target, CONST0_RTX (mode), vl}; > + rtx ops[] = {target, CONST0_RTX (mode), vl}; > riscv_vector::emit_vlmax_insn (code_for_pred_mov (mode), > riscv_vector::RVV_UNOP, ops); Reviewed-by: Palmer Dabbelt as both cleanups look better to me. Thanks!