On Mon, 2024-08-05 at 14:15 -0700, Andrew Pinski wrote:
> This was an interesting compare debug failure to debug. The first symptom
> was in gcse which would produce different order of creating psedu-registers.
> This
> was caused by a different order of a hashtable walk, due to the hash table
> having different
> number of entries. Which in turn was due to the number of max insn being
> different between
> the 2 runs. The place max insn uid comes from was in sh_recog_treg_set_expr
> which is called
> via rtx_costs and fwprop would cause rtx_costs in some cases for debug insn
> related stuff.
>
> Build and tested for sh4-linux-gnu.
Thanks so much!
I think it should be safe to install this on all open branches.
Best regards,
Oleg Endo
>
> PR target/116189
>
> gcc/ChangeLog:
>
> * config/sh/sh.cc (sh_recog_treg_set_expr): Don't call make_insn_raw,
> make the insn with a fake uid.
>
> gcc/testsuite/ChangeLog:
>
> * c-c++-common/torture/pr116189-1.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
> gcc/config/sh/sh.cc | 12 +++++++-
> .../c-c++-common/torture/pr116189-1.c | 30 +++++++++++++++++++
> 2 files changed, 41 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/c-c++-common/torture/pr116189-1.c
>
> diff --git a/gcc/config/sh/sh.cc b/gcc/config/sh/sh.cc
> index bc017420381..7391b8df583 100644
> --- a/gcc/config/sh/sh.cc
> +++ b/gcc/config/sh/sh.cc
> @@ -12297,7 +12297,17 @@ sh_recog_treg_set_expr (rtx op, machine_mode mode)
> have to capture its current state and restore it afterwards. */
> recog_data_d prev_recog_data = recog_data;
>
> - rtx_insn* i = make_insn_raw (gen_rtx_SET (get_t_reg_rtx (), op));
> + /* Note we can't use insn_raw here since that increases the uid
> + and could cause debug compare differences; this insn never leaves
> + this function so create a dummy one. */
> + rtx_insn* i = as_a <rtx_insn *> (rtx_alloc (INSN));
> +
> + INSN_UID (i) = 1;
> + PATTERN (i) = gen_rtx_SET (get_t_reg_rtx (), op);
> + INSN_CODE (i) = -1;
> + REG_NOTES (i) = NULL;
> + INSN_LOCATION (i) = curr_insn_location ();
> + BLOCK_FOR_INSN (i) = NULL;
> SET_PREV_INSN (i) = NULL;
> SET_NEXT_INSN (i) = NULL;
>
> diff --git a/gcc/testsuite/c-c++-common/torture/pr116189-1.c
> b/gcc/testsuite/c-c++-common/torture/pr116189-1.c
> new file mode 100644
> index 00000000000..055c563f43e
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/torture/pr116189-1.c
> @@ -0,0 +1,30 @@
> +/* { dg-additional-options "-fcompare-debug" } */
> +
> +/* PR target/116189 */
> +
> +/* In the sh backend, we used to create insn in the path of rtx_costs.
> + This means sometimes the max uid for insns would be different between
> + debugging and non debugging which then would cause gcse's hashtable
> + to have different number of slots which would cause a different walk
> + for that hash table. */
> +
> +extern void ff(void);
> +extern short nn[8][4];
> +typedef unsigned short move_table[4];
> +extern signed long long ira_overall_cost;
> +extern signed long long ira_load_cost;
> +extern move_table *x_ira_register_move_cost[1];
> +struct move { struct move *next; };
> +unsigned short t;
> +void emit_move_list(struct move * list, int freq, unsigned char mode, int
> regno) {
> + int cost;
> + for (; list != 0; list = list->next)
> + {
> + ff();
> + unsigned short aclass = t;
> + cost = (nn)[mode][aclass] ;
> + ira_load_cost = cost;
> + cost = x_ira_register_move_cost[mode][aclass][aclass] * freq ;
> + ira_overall_cost = cost;
> + }
> +}
> --
> 2.43.0
>
