GCSE: Export add_label_notes as global function
Checks
Commit Message
From: Ju-Zhe Zhong <juzhe.zhong@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.
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(-)
Comments
On Mon, 10 Jul 2023, juzhe.zhong@rivai.ai wrote:
> From: Ju-Zhe Zhong <juzhe.zhong@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
>
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 ?
Thanks.
juzhe.zhong@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.zhong@rivai.ai wrote:
> From: Ju-Zhe Zhong <juzhe.zhong@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 <rguenther@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)
Sorry, I forget to add the patch:
https://gcc.gnu.org/pipermail/gcc-patches/2023-July/623960.html
juzhe.zhong@rivai.ai
From: juzhe.zhong@rivai.ai
Date: 2023-07-10 15:58
To: rguenther
CC: gcc-patches; jeffreyalaw
Subject: Re: Re: [PATCH] GCSE: Export add_label_notes as global function
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 ?
Thanks.
juzhe.zhong@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.zhong@rivai.ai wrote:
> From: Ju-Zhe Zhong <juzhe.zhong@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 <rguenther@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)
On Mon, 10 Jul 2023, juzhe.zhong@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.zhong@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.zhong@rivai.ai wrote:
>
> > From: Ju-Zhe Zhong <juzhe.zhong@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
> >
>
>
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.zhong@rivai.ai
From: Richard Biener
Date: 2023-07-10 16:01
To: juzhe.zhong@rivai.ai
CC: gcc-patches; jeffreyalaw
Subject: Re: Re: [PATCH] GCSE: Export add_label_notes as global function
On Mon, 10 Jul 2023, juzhe.zhong@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.zhong@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.zhong@rivai.ai wrote:
>
> > From: Ju-Zhe Zhong <juzhe.zhong@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 <rguenther@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)
@@ -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. */
@@ -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);
@@ -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