Thanks Richi.
I have sent V2 patch:
https://gcc.gnu.org/pipermail/gcc-patches/2023-July/623964.html
which is fixing the comments for you:
-/* Add EXPR to the end of basic block BB.
-
- This is used by both the PRE and code hoisting. */
+/* Return the INSN which is added at the end of the block BB with
+ same instruction pattern with PAT. */
+rtx_insn *
+insert_insn_end_basic_block (rtx_insn *pat, basic_block bb)
Is it better ?
Thanks.
juzhe.zh...@rivai.ai
From: Richard Biener
Date: 2023-07-10 16:01
To: juzhe.zh...@rivai.ai
CC: gcc-patches; jeffreyalaw
Subject: Re: Re: [PATCH] GCSE: Export add_label_notes as global function
On Mon, 10 Jul 2023, juzhe.zh...@rivai.ai wrote:
> Hi, Richard.
>
> I find out I just only need to export 'insert_insn_end_basic_block' for
> global used by RISC-V port (current riscv-vsetvl.cc and future riscv.cc).
>
> Does it look more reasonable ?
Yes, it looks more reasonable - I'll leave review to somebody knowing
the code - it would be nice to better document the API rather than
saying that it's used by gcse and code hoisting ...
> Thanks.
>
>
> juzhe.zh...@rivai.ai
>
> From: Richard Biener
> Date: 2023-07-10 15:25
> To: Ju-Zhe Zhong
> CC: gcc-patches; jeffreyalaw
> Subject: Re: [PATCH] GCSE: Export add_label_notes as global function
> On Mon, 10 Jul 2023, juzhe.zh...@rivai.ai wrote:
>
> > From: Ju-Zhe Zhong <juzhe.zh...@rivai.ai>
> >
> > Since 'add_lable_notes' is a generic helper function which is used by
> > riscv-vsetvl.cc
> > in RISC-V port backend. And it's also will be used by riscv.cc too by the
> > following patches.
> > Export it as global helper function.
>
> I know nothing about this code but grepping shows me the existing
> rebuild_jump_labels () API which also properly resets LABEL_NUSES
> before incrementing it. I don't think exporting add_label_notes ()
> as-is is good because it at least will wreck those counts.
> GCSE uses this function to add the notes for a specific instruction
> only, so if we want to export such API the name of the function
> should imply it works on a single insn.
>
> Richard.
>
> > gcc/ChangeLog:
> >
> > * config/riscv/riscv-vsetvl.cc (add_label_notes): Remove it.
> > * gcse.cc (add_label_notes): Export it as global.
> > (one_pre_gcse_pass): Ditto.
> > * gcse.h (add_label_notes): Ditto.
> >
> > ---
> > gcc/config/riscv/riscv-vsetvl.cc | 48 +-------------------------------
> > gcc/gcse.cc | 3 +-
> > gcc/gcse.h | 1 +
> > 3 files changed, 3 insertions(+), 49 deletions(-)
> >
> > diff --git a/gcc/config/riscv/riscv-vsetvl.cc
> > b/gcc/config/riscv/riscv-vsetvl.cc
> > index ab47901e23f..038ba22362e 100644
> > --- a/gcc/config/riscv/riscv-vsetvl.cc
> > +++ b/gcc/config/riscv/riscv-vsetvl.cc
> > @@ -98,6 +98,7 @@ along with GCC; see the file COPYING3. If not see
> > #include "lcm.h"
> > #include "predict.h"
> > #include "profile-count.h"
> > +#include "gcse.h"
> > #include "riscv-vsetvl.h"
> >
> > using namespace rtl_ssa;
> > @@ -763,53 +764,6 @@ insert_vsetvl (enum emit_type emit_type, rtx_insn
> > *rinsn,
> > return VSETVL_DISCARD_RESULT;
> > }
> >
> > -/* If X contains any LABEL_REF's, add REG_LABEL_OPERAND notes for them
> > - to INSN. If such notes are added to an insn which references a
> > - CODE_LABEL, the LABEL_NUSES count is incremented. We have to add
> > - that note, because the following loop optimization pass requires
> > - them. */
> > -
> > -/* ??? If there was a jump optimization pass after gcse and before loop,
> > - then we would not need to do this here, because jump would add the
> > - necessary REG_LABEL_OPERAND and REG_LABEL_TARGET notes. */
> > -
> > -static void
> > -add_label_notes (rtx x, rtx_insn *rinsn)
> > -{
> > - enum rtx_code code = GET_CODE (x);
> > - int i, j;
> > - const char *fmt;
> > -
> > - if (code == LABEL_REF && !LABEL_REF_NONLOCAL_P (x))
> > - {
> > - /* This code used to ignore labels that referred to dispatch tables
> > to
> > - avoid flow generating (slightly) worse code.
> > -
> > - We no longer ignore such label references (see LABEL_REF handling in
> > - mark_jump_label for additional information). */
> > -
> > - /* There's no reason for current users to emit jump-insns with
> > - such a LABEL_REF, so we don't have to handle REG_LABEL_TARGET
> > - notes. */
> > - gcc_assert (!JUMP_P (rinsn));
> > - add_reg_note (rinsn, REG_LABEL_OPERAND, label_ref_label (x));
> > -
> > - if (LABEL_P (label_ref_label (x)))
> > - LABEL_NUSES (label_ref_label (x))++;
> > -
> > - return;
> > - }
> > -
> > - for (i = GET_RTX_LENGTH (code) - 1, fmt = GET_RTX_FORMAT (code); i >= 0;
> > i--)
> > - {
> > - if (fmt[i] == 'e')
> > - add_label_notes (XEXP (x, i), rinsn);
> > - else if (fmt[i] == 'E')
> > - for (j = XVECLEN (x, i) - 1; j >= 0; j--)
> > - add_label_notes (XVECEXP (x, i, j), rinsn);
> > - }
> > -}
> > -
> > /* Add EXPR to the end of basic block BB.
> >
> > This is used by both the PRE and code hoisting. */
> > diff --git a/gcc/gcse.cc b/gcc/gcse.cc
> > index 72832736572..5627fbf127a 100644
> > --- a/gcc/gcse.cc
> > +++ b/gcc/gcse.cc
> > @@ -483,7 +483,6 @@ static void pre_insert_copies (void);
> > static int pre_delete (void);
> > static int pre_gcse (struct edge_list *);
> > static int one_pre_gcse_pass (void);
> > -static void add_label_notes (rtx, rtx_insn *);
> > static void alloc_code_hoist_mem (int, int);
> > static void free_code_hoist_mem (void);
> > static void compute_code_hoist_vbeinout (void);
> > @@ -2639,7 +2638,7 @@ one_pre_gcse_pass (void)
> > then we would not need to do this here, because jump would add the
> > necessary REG_LABEL_OPERAND and REG_LABEL_TARGET notes. */
> >
> > -static void
> > +void
> > add_label_notes (rtx x, rtx_insn *insn)
> > {
> > enum rtx_code code = GET_CODE (x);
> > diff --git a/gcc/gcse.h b/gcc/gcse.h
> > index 5582b29eec2..e5ee9b088bd 100644
> > --- a/gcc/gcse.h
> > +++ b/gcc/gcse.h
> > @@ -41,5 +41,6 @@ extern struct target_gcse *this_target_gcse;
> >
> > void gcse_cc_finalize (void);
> > extern bool gcse_or_cprop_is_too_expensive (const char *);
> > +void add_label_notes (rtx, rtx_insn *);
> >
> > #endif
> >
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
