| Message ID | 20240805211553.3743105-1-quic_apinski@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | sh: Don't call make_insn_raw in sh_recog_treg_set_expr [PR116189] | expand |
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_apinski@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 >
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; + } +}
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. 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_apinski@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