From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk1-xa2b.google.com (mail-vk1-xa2b.google.com [IPv6:2607:f8b0:4864:20::a2b]) by sourceware.org (Postfix) with ESMTPS id 4737A3858D32 for ; Mon, 29 May 2023 03:05:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4737A3858D32 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-vk1-xa2b.google.com with SMTP id 71dfb90a1353d-456f1cc1791so785267e0c.2 for ; Sun, 28 May 2023 20:05:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1685329511; x=1687921511; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=xxYc1lkbyBCrYyuyzw6cAXG3MSCC3F7Zs7iTjdkHujY=; b=sjBovK3N91DXy2j6llHxY0yaFIWDARBNofflRwoCSWDBOImKS1gUouTU/fSay2RGAa aYOn1lVdZgajg0R1fQMBspY3U4z0mz+LQVkPDiAV3J7f7klL/HVexemYoYW+aw4XXuQg X04PkQshhUAzvMiO8Uyn3+K4TbtNO2NrA3yG7XXCyoY8xlnjTm+uhk0r5e0hRWRr5bqc vQRTmlPph0X4nUFs67lD/hxjY0drucpBCnNt4TuX8wlsb0FK32oQyrVW/A22MJ2XqdHZ gitAUAbH76j9qEqhvO480Gr9dAORlqgYJb6XqEQNiojkvwsx/dUlTQA6Aiso+hlySGzh rgiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685329511; x=1687921511; 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=xxYc1lkbyBCrYyuyzw6cAXG3MSCC3F7Zs7iTjdkHujY=; b=XCw1QIDJ2TZYRKbFoxLksqa59EoIdLJWTWD0rly/wV6Or+nkihFUP7gkQUnxBhDRd2 iVYeqT1TlSfVAcFXZOfjvj5/2PmJnNFW0Lvz8Ax4GDk1pw3N4g93lk9V+IF3FQpAmv0D +VoN0ElYlXAJq8cB/cW4IVR5SXIav+8h3GUrSG3O2KDOnaQw5+YhhT151W1P/Y818ZXP Z8mbkCrDIjK/RIepR0S8mMHlKNQ2KqxtCmv8anG6BoWgjuci5YL3UXwABCJ9qgVwO/YV PJDszdC4w1RKGLVaBgCcalWhutVZo5sRFrpsw4sdm98LlOOyVlz637ygONLAXvqFtpNH QU+Q== X-Gm-Message-State: AC+VfDyOgSzXH/sb1nZ2SamBqfhD1JUQFkhpzIH5quVwGcd3aswiDLyQ o5YjclSg7aD5Bf/vWCyi7oIQoLHzdzrDjcEkWig= X-Google-Smtp-Source: ACHHUZ7mAJNV06VtFh/xWo3rXXDsb749xPM7k2zZ4SzBC4dP9nQ+bCb9K7vn2PXQdSBoL/0AAutTXNENFkjaNGvloKM= X-Received: by 2002:a1f:5f91:0:b0:453:8523:c9d8 with SMTP id t139-20020a1f5f91000000b004538523c9d8mr1960735vkb.0.1685329511153; Sun, 28 May 2023 20:05:11 -0700 (PDT) MIME-Version: 1.0 References: <20230512090443.34123-1-gaofei@eswincomputing.com> <20230512090443.34123-2-gaofei@eswincomputing.com> In-Reply-To: <20230512090443.34123-2-gaofei@eswincomputing.com> From: Kito Cheng Date: Mon, 29 May 2023 11:05:00 +0800 Message-ID: Subject: Re: [PATCH 1/1] [V2] [RISC-V] support cm.push cm.pop cm.popret in zcmp To: Fei Gao Cc: gcc-patches@gcc.gnu.org, palmer@dabbelt.com, jeffreyalaw@gmail.com, sinan.lin@linux.alibaba.com, jiawei@iscas.ac.cn Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.0 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,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: Thanks for this patch, just few minor comment, I think this is pretty close to accept :) Could you reference JiaWei's match_parallel[1] to prevent adding bunch of *_offset_operand and stack_push_up_to_*_operand? [1] https://patchwork.sourceware.org/project/gcc/patch/20230406062118.47431-5-jiawei@iscas.ac.cn/ > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > index 629e5e45cac..a0a2db1f594 100644 > --- a/gcc/config/riscv/riscv.cc > +++ b/gcc/config/riscv/riscv.cc > @@ -117,6 +117,14 @@ struct GTY(()) riscv_frame_info { > /* How much the GPR save/restore routines adjust sp (or 0 if unused). */ > unsigned save_libcall_adjustment; > > + /* the minimum number of bytes, in multiples of 16-byte address increments, > + required to cover the registers in a multi push & pop. */ > + unsigned multi_push_adj_base; > + > + /* the number of additional 16-byte address increments allocated for the stack frame > + in a multi push & pop. */ > + unsigned multi_push_adj_addi; > + > /* Offsets of fixed-point and floating-point save areas from frame bottom */ > poly_int64 gp_sp_offset; > poly_int64 fp_sp_offset; > @@ -413,6 +421,21 @@ static const struct riscv_tune_info riscv_tune_info_table[] = { > #include "riscv-cores.def" > }; > > +typedef enum > +{ > + SI_IDX = 0, > + DI_IDX, > + MAX_MODE_IDX = DI_IDX > +} mode_idx; > + Didn't see any use in this version? > @@ -5574,18 +5924,25 @@ riscv_expand_epilogue (int style) > REG_NOTES (insn) = dwarf; > } > > - if (use_restore_libcall) > - frame->mask = 0; /* Temporarily fib for GPRs. */ > + if (use_restore_libcall || use_multi_pop) > + frame->mask = 0; /* Temporarily fib that we need not save GPRs. */ > > /* If we need to restore registers, deallocate as much stack as > possible in the second step without going out of range. */ > - if ((frame->mask | frame->fmask) != 0) > + if (use_multi_pop) > + { > + if (frame->fmask > + && known_gt (frame->total_size - multipop_size, > + frame->frame_pointer_offset)) > + step2 = riscv_first_stack_step (frame, frame->total_size - multipop_size); > + } > + else if ((frame->mask | frame->fmask) != 0) > step2 = riscv_first_stack_step (frame, frame->total_size - libcall_size); > > - if (use_restore_libcall) > + if (use_restore_libcall || use_multi_pop) > frame->mask = mask; /* Undo the above fib. */ > > - poly_int64 step1 = frame->total_size - step2 - libcall_size; > + poly_int64 step1 = frame->total_size - step2 - libcall_size - multipop_size ; > > /* Set TARGET to BASE + STEP1. */ > if (known_gt (step1, 0)) > @@ -5620,7 +5977,7 @@ riscv_expand_epilogue (int style) > adjust)); > rtx dwarf = NULL_RTX; > rtx cfa_adjust_rtx = gen_rtx_PLUS (Pmode, stack_pointer_rtx, > - GEN_INT (step2)); > + GEN_INT (step2 + libcall_size + multipop_size)); Why we need `+ libcall_size` here? or...why we don't need that before? > > dwarf = alloc_reg_note (REG_CFA_DEF_CFA, cfa_adjust_rtx, dwarf); > RTX_FRAME_RELATED_P (insn) = 1; > @@ -5635,15 +5992,15 @@ riscv_expand_epilogue (int style) > epilogue_cfa_sp_offset = step2; > } > > - if (use_restore_libcall) > + if (use_restore_libcall || use_multi_pop) > frame->mask = 0; /* Temporarily fib that we need not save GPRs. */ > > /* Restore the registers. */ > - riscv_for_each_saved_reg (frame->total_size - step2 - libcall_size, > + riscv_for_each_saved_reg (frame->total_size - step2 - libcall_size - multipop_size, > riscv_restore_reg, > true, style == EXCEPTION_RETURN); > > - if (use_restore_libcall) > + if (use_restore_libcall || use_multi_pop) > frame->mask = mask; /* Undo the above fib. */ > > if (need_barrier_p) > @@ -5657,14 +6014,30 @@ riscv_expand_epilogue (int style) > > rtx dwarf = NULL_RTX; > rtx cfa_adjust_rtx = gen_rtx_PLUS (Pmode, stack_pointer_rtx, > - const0_rtx); > + GEN_INT (libcall_size + multipop_size)); Same question for `libcall_size` part.