How to update reg_dead notes
Hi, in order to fix PR64331 I tried to implement new target-specific passes whose sole purpose is to recompute REG_DEAD notes. The avr BE relies on correct dead notes which are used in avr.c:reg_unused_after which uses dead_or_set_p. avr BE needs correct dead notes in ADJUST_INSN_LENGTH, get_attr_length, get_attr_min_length, etc. After trying for more than one day I am really frustrated; each approach ended up in seg_faults somewhere in df. Following the source comments in df-core.c, recomputing dead notes should be as easy as df_note_add_problem (); df_analyze (); in the execute() method of the new pass. As this (and many many other tries using df_scan_alloc, df_scan_blocks df_finish_pass, df_insn_rescan_all, etc.) always crashes the compiler, I must do something completely wrong... Could you give me some advice on correct usage of df or even more preferred point me to a comprehensible documentation of df which is more complete than in df-core.c? Internals don't treat df, and the source comments are not really helpful, e.g. the complete documentation of df_analyze is /* Analyze dataflow info. */. Not a single word about prerequisites (except that it must run after df_init and before df_finish and needs correct cfg). One example of a crash is that df->insns[uid] is being accessed and dereferenced where uid is a valid uid but df->insns[uid] is NULL. df-core.c mentions "given instances of df". How do I get one? The only instance I can find is the global struct df_f *df. Does this mean one has to mess around with that global variable?
Re: How to update reg_dead notes
It is generally as easy as just adding the problem and calling df_analyze. You tend to get into trouble if the rtl is not good, i.e. there is improper sharing or other violations of the canonical rtl rules. DF does not like improperly shared rtl and it has not been uncommon for port specific passes to play loose with these rules. I would suggest that first you try a build where you turn on all of the rtl checking and see if that comes out clean. Kenny On 02/24/2015 06:41 AM, Georg-Johann Lay wrote: Hi, in order to fix PR64331 I tried to implement new target-specific passes whose sole purpose is to recompute REG_DEAD notes. The avr BE relies on correct dead notes which are used in avr.c:reg_unused_after which uses dead_or_set_p. avr BE needs correct dead notes in ADJUST_INSN_LENGTH, get_attr_length, get_attr_min_length, etc. After trying for more than one day I am really frustrated; each approach ended up in seg_faults somewhere in df. Following the source comments in df-core.c, recomputing dead notes should be as easy as df_note_add_problem (); df_analyze (); in the execute() method of the new pass. As this (and many many other tries using df_scan_alloc, df_scan_blocks df_finish_pass, df_insn_rescan_all, etc.) always crashes the compiler, I must do something completely wrong... Could you give me some advice on correct usage of df or even more preferred point me to a comprehensible documentation of df which is more complete than in df-core.c? Internals don't treat df, and the source comments are not really helpful, e.g. the complete documentation of df_analyze is /* Analyze dataflow info. */. Not a single word about prerequisites (except that it must run after df_init and before df_finish and needs correct cfg). One example of a crash is that df->insns[uid] is being accessed and dereferenced where uid is a valid uid but df->insns[uid] is NULL. df-core.c mentions "given instances of df". How do I get one? The only instance I can find is the global struct df_f *df. Does this mean one has to mess around with that global variable?
Re: How to update reg_dead notes
On Tue, 2015-02-24 at 16:59 +0100, Georg-Johann Lay wrote:
It doesn't really answer your question, but just as a side note, the
following ...
> + struct register_pass_info insert_before_bbro =
> +{
> + notes_bbro_pass, /* pass */
> + "bbro", /* reference_pass_name */
> + 1,/* ref_pass_instance_number */
> + PASS_POS_INSERT_BEFORE/* position */
> +};
> +
> + struct register_pass_info insert_before_compgotos =
> +{
> + notes_compgotos_pass, /* pass */
> + "compgotos", /* reference_pass_name */
> + 1,/* ref_pass_instance_number */
> + PASS_POS_INSERT_BEFORE/* position */
> +};
> +
> + struct register_pass_info insert_before_shorten =
> +{
> + notes_shorten_pass, /* pass */
> + "shorten",/* reference_pass_name */
> + 1,/* ref_pass_instance_number */
> + PASS_POS_INSERT_BEFORE/* position */
> +};
> +
> + struct register_pass_info insert_before_final =
> +{
> + notes_final_pass, /* pass */
> + "final", /* reference_pass_name */
> + 1,/* ref_pass_instance_number */
> + PASS_POS_INSERT_BEFORE/* position */
> +};
> +
> + register_pass (&insert_before_bbro);
> + register_pass (&insert_before_compgotos);
> + register_pass (&insert_before_shorten);
> + register_pass (&insert_before_final);
... can be done in 4 lines of code, without all the struct stuff to pass
function arguments. See also "register_pass" calls in config/sh/sh.c.
Cheers,
Oleg
Re: How to update reg_dead notes
Am 02/24/2015 um 02:11 PM schrieb Kenneth Zadeck:
On 02/24/2015 06:41 AM, Georg-Johann Lay wrote:
Hi, in order to fix PR64331 I tried to implement new target-specific passes
whose sole purpose is to recompute REG_DEAD notes.
The avr BE relies on correct dead notes which are used in
avr.c:reg_unused_after which uses dead_or_set_p. avr BE needs correct dead
notes in ADJUST_INSN_LENGTH, get_attr_length, get_attr_min_length, etc.
After trying for more than one day I am really frustrated; each approach
ended up in seg_faults somewhere in df.
Following the source comments in df-core.c, recomputing dead notes should be
as easy as
df_note_add_problem ();
df_analyze ();
in the execute() method of the new pass.
As this (and many many other tries using df_scan_alloc, df_scan_blocks
df_finish_pass, df_insn_rescan_all, etc.) always crashes the compiler, I must
do something completely wrong...
Could you give me some advice on correct usage of df or even more preferred
point me to a comprehensible documentation of df which is more complete than
in df-core.c?
Hi, thanks for answering.
It is generally as easy as just adding the problem and calling df_analyze.
You tend to get into trouble if the rtl is not good, i.e. there is improper
sharing or other violations of the canonical rtl rules. DF does not like
improperly shared rtl and it has not been uncommon for port specific passes to
play loose with these rules.
Currently there are no port-specific passes. The only port-specific passes are
the new passes which I am trying to add in order to keep REG_DEAD notes up to date.
I would suggest that first you try a build where you turn on all of the rtl
checking and see if that comes out clean.
Kenny
Okay, here we go. I configured avr-gcc from recent 4.9-branch with
--enable-checking=all on x86-linux-gnu:
$ ../../gcc.gnu.org/gcc-4_9-branch/configure --target=avr
--prefix=/local/gnu/install/gcc-4.9 --disable-nls --with-dwarf2
--enable-target-optspace=yes --with-gnu-ld --with-gnu-as
--enable-languages=c,c++ --enable-checking=all
The passes are added by means of the attached patch and installed right before
passes which might use avr.c:reg_unused_after, i.e. passes that query for
(minimum) insn lengths: .bbro, .compgotos, .shorten and .final.
The compiler crashes when it builds libgcc. The test case is attached as
libgcc2-addvsi3.c and compiled as
$ path-to-xgcc/xgcc -B path-to-xgcc -S libgcc2-addvsi3.c
The crash message (same with optimization) is for the pass which has been added
just before .shorten:
libgcc2-addvsi3.c: In function '__addvhi3':
libgcc2-addvsi3.c:1514:1: internal compiler error: in df_refs_verify, at
df-scan.c:4323
}
^
0x82dfe8a df_refs_verify
../../../gcc.gnu.org/gcc-4_9-branch/gcc/df-scan.c:4323
0x82e7b34 df_insn_refs_verify
../../../gcc.gnu.org/gcc-4_9-branch/gcc/df-scan.c:4414
0x82e9906 df_bb_verify
../../../gcc.gnu.org/gcc-4_9-branch/gcc/df-scan.c:4441
0x82e9d8e df_scan_verify()
../../../gcc.gnu.org/gcc-4_9-branch/gcc/df-scan.c:4573
0x82d3787 df_verify()
../../../gcc.gnu.org/gcc-4_9-branch/gcc/df-core.c:1862
0x82d3809 df_analyze_1
../../../gcc.gnu.org/gcc-4_9-branch/gcc/df-core.c:1250
0x896188b avr_pass_recompute_notes::execute()
../../../gcc.gnu.org/gcc-4_9-branch/gcc/config/avr/avr.c:317
Please submit a full bug report,
Johann
Index: config/avr/avr.c
===
--- config/avr/avr.c (revision 220738)
+++ config/avr/avr.c (working copy)
@@ -51,6 +51,8 @@
#include "target-def.h"
#include "params.h"
#include "df.h"
+#include "context.h"
+#include "tree-pass.h" /* for current_pass */
/* Maximal allowed offset for an address in the LD command */
#define MAX_LD_OFFSET(MODE) (64 - (signed)GET_MODE_SIZE (MODE))
@@ -284,6 +286,48 @@ avr_to_int_mode (rtx x)
: simplify_gen_subreg (int_mode_for_mode (mode), x, mode, 0);
}
+
+static const pass_data avr_pass_data_recompute_notes =
+{
+ RTL_PASS, /* type */
+ "avr-notes1", /* name */
+ OPTGROUP_NONE, /* optinfo_flags */
+ false, /* has_gate */
+ true, /* has_execute */
+ TV_DF_SCAN, /* tv_id */
+ 0, /* properties_required */
+ 0, /* properties_provided */
+ 0, /* properties_destroyed */
+ 0, /* todo_flags_start */
+ 0, /* todo_flags_finish */
+};
+
+
+class avr_pass_recompute_notes : public rtl_opt_pass
+{
+public:
+ avr_pass_recompute_notes (gcc::context *ctxt)
+: rtl_opt_pass (avr_pass_data_recompute_notes, ctxt)
+ {}
+
+ /* opt_pass methods */
+ unsigned int execute ()
+ {
+df_note_add_problem ();
+df_analyze ();
+
+return 0;
+ }
+}; // avr_pass_recompute_notes
+
+
+static rtl_opt_pass*
+avr_make_pass_recompute_notes (gcc::context *ctxt, const char *name)
+{
+ rtl_opt_pass *pass = new avr_pass_recompute_notes (ctxt);
+ pass->name = name;
+ return pass;
+}
/* Implement `TARGET_OPTION_OVERRIDE'. */
@@ -346,6 +390,58 @@ avr_option_override (vo
Re: How to update reg_dead notes
> Could you give me some advice on correct usage of df or even more preferred > point me to a comprehensible documentation of df which is more complete than > in df-core.c? Take a look at the c6x and mep ports. -- Eric Botcazou
Re: How to update reg_dead notes
Am 02/24/2015 um 06:06 PM schrieb Eric Botcazou:
Could you give me some advice on correct usage of df or even more preferred
point me to a comprehensible documentation of df which is more complete than
in df-core.c?
Take a look at the c6x and mep ports.
Thanks for the hint. I changed the execute method to:
unsigned int execute ()
{
compute_bb_for_insn ();
//df_clear_flags (DF_LR_RUN_DCE);
df_note_add_problem ();
df_analyze ();
df_finish_pass (false);
return 0;
}
bit it keeps aborting. Actually I am just copy pasting code from some other
passes / BEs, but these places also don't have explanation for why they must
use, may use, must not use specific functions.
Compiling the mentioned test case from libgcc with -Os (so that less insns are
left over) and patching df-scan.c:df_refs_verify to print the refs just before
it does gcc_assert (0):
new_ref = { u-1(18){ }}
*old_rec = { u-1(18){ }u-1(19){ }u-1(24){ }u-1(25){ }}
libgcc2-addvsi3.c: In function '__addvhi3':
libgcc2-addvsi3.c:1514:1: internal compiler error: in df_refs_verify, at
df-scan.c:4338
In df_insn_refs_verify which is in the call chain of df_refs_verify, insn reads:
(insn 21 19 22 5 (parallel [
(set (cc0)
(compare (reg/v:HI 18 r18 [orig:48 a ] [48])
(reg/v:HI 24 r24 [orig:46 w ] [46])))
(clobber (scratch:QI))
]) libgcc2-addvsi3.c:1511 408 {*cmphi}
(expr_list:REG_DEAD (reg/v:HI 18 r18 [orig:48 a ] [48])
(nil)))
r18 and r24 are ordinary general-purpose hard registers.
The latest pass which runs before the crash is .split5, i.e.
recog.c::pass_split_for_shorten_branches which executes split_all_insns_noflow
which in turn reads:
/* Same as split_all_insns, but do not expect CFG to be available.
Used by machine dependent reorg passes. */
unsigned int
split_all_insns_noflow (void)
{ ...
Does this mean CFG is (or might be) messed up after this pass and this is the
reason for why df crashes because df needs correct CFG?
Johann
Re: How to update reg_dead notes
when i suggested that you do a build with all of the checking turned on,
i wanted you to this without you new code in it.there is a good
possibility that the problem is that your port is generating bad rtl.
Also, you should generate a debuggable compiler so that the line numbers
have some relevance to reality.
On 02/24/2015 01:23 PM, Georg-Johann Lay wrote:
Am 02/24/2015 um 06:06 PM schrieb Eric Botcazou:
Could you give me some advice on correct usage of df or even more
preferred
point me to a comprehensible documentation of df which is more
complete than
in df-core.c?
Take a look at the c6x and mep ports.
Thanks for the hint. I changed the execute method to:
unsigned int execute ()
{
compute_bb_for_insn ();
//df_clear_flags (DF_LR_RUN_DCE);
df_note_add_problem ();
df_analyze ();
df_finish_pass (false);
return 0;
}
bit it keeps aborting. Actually I am just copy pasting code from some
other passes / BEs, but these places also don't have explanation for
why they must use, may use, must not use specific functions.
Compiling the mentioned test case from libgcc with -Os (so that less
insns are left over) and patching df-scan.c:df_refs_verify to print
the refs just before it does gcc_assert (0):
new_ref = { u-1(18){ }}
*old_rec = { u-1(18){ }u-1(19){ }u-1(24){ }u-1(25){ }}
libgcc2-addvsi3.c: In function '__addvhi3':
libgcc2-addvsi3.c:1514:1: internal compiler error: in df_refs_verify,
at df-scan.c:4338
In df_insn_refs_verify which is in the call chain of df_refs_verify,
insn reads:
(insn 21 19 22 5 (parallel [
(set (cc0)
(compare (reg/v:HI 18 r18 [orig:48 a ] [48])
(reg/v:HI 24 r24 [orig:46 w ] [46])))
(clobber (scratch:QI))
]) libgcc2-addvsi3.c:1511 408 {*cmphi}
(expr_list:REG_DEAD (reg/v:HI 18 r18 [orig:48 a ] [48])
(nil)))
r18 and r24 are ordinary general-purpose hard registers.
The latest pass which runs before the crash is .split5, i.e.
recog.c::pass_split_for_shorten_branches which executes
split_all_insns_noflow which in turn reads:
/* Same as split_all_insns, but do not expect CFG to be available.
Used by machine dependent reorg passes. */
unsigned int
split_all_insns_noflow (void)
{ ...
Does this mean CFG is (or might be) messed up after this pass and this
is the reason for why df crashes because df needs correct CFG?
Johann
Re: How to update reg_dead notes
On Tue, 2015-02-24 at 19:23 +0100, Georg-Johann Lay wrote:
>
> The latest pass which runs before the crash is .split5, i.e.
> recog.c::pass_split_for_shorten_branches which executes
> split_all_insns_noflow
> which in turn reads:
>
> /* Same as split_all_insns, but do not expect CFG to be available.
> Used by machine dependent reorg passes. */
>
> unsigned int
> split_all_insns_noflow (void)
> { ...
>
> Does this mean CFG is (or might be) messed up after this pass and this is the
> reason for why df crashes because df needs correct CFG?
Yes, the CFG is only valid until some point. E.g. on SH it stops being
valid after the machine reorg pass the latest (PR 59189). Doing
anything CFG related after split5 might not be a good idea.
Cheers,
Oleg
Re: How to update reg_dead notes
Am 02/24/2015 um 07:33 PM schrieb Kenneth Zadeck:
when i suggested that you do a build with all of the checking turned on, i
wanted you to this without you new code in it.there is a good possibility
that the problem is that your port is generating bad rtl.
Ah, ok. This actually revealed an ice-checking; but that was not related to
the df problems...
I think the problem was that I tried to run passes needing df after free_cfg.
My current solution works so far: It fixes the test case (correct code + insn
lengths) and it did not ICE up to now.
Stuff is running a bit slow with all checks on so building and running tests
will take a while...
FYI, you find my current patch attached.
Thanks for your help and time,
Johann
Index: config/avr/avr.c
===
--- config/avr/avr.c(revision 220963)
+++ config/avr/avr.c(working copy)
@@ -51,6 +51,8 @@
#include "target-def.h"
#include "params.h"
#include "df.h"
+#include "context.h"
+#include "tree-pass.h"
/* Maximal allowed offset for an address in the LD command */
#define MAX_LD_OFFSET(MODE) (64 - (signed)GET_MODE_SIZE (MODE))
@@ -285,6 +287,58 @@ avr_to_int_mode (rtx x)
}
+static const pass_data avr_pass_data_recompute_notes =
+{
+ RTL_PASS, /* type */
+ "avr-xx", /* name (will be patched) */
+ OPTGROUP_NONE, /* optinfo_flags */
+ false, /* has_gate */
+ true, /* has_execute */
+ TV_DF_SCAN, /* tv_id */
+ 0, /* properties_required */
+ 0, /* properties_provided */
+ 0, /* properties_destroyed */
+ 0, /* todo_flags_start */
+ (TODO_df_finish | TODO_verify_rtl_sharing
+ | TODO_verify_flow ), /* todo_flags_finish */
+};
+
+
+class avr_pass_recompute_notes : public rtl_opt_pass
+{
+public:
+ avr_pass_recompute_notes (gcc::context *ctxt, const char *pass_name)
+: rtl_opt_pass (avr_pass_data_recompute_notes, ctxt)
+ {
+name = pass_name;
+ }
+
+ unsigned int execute ()
+ {
+df_note_add_problem ();
+df_analyze ();
+
+return 0;
+ }
+}; // avr_pass_recompute_notes
+
+
+static void
+avr_register_passes (void)
+{
+ /* This avr-specific pass (re)computes insn notes, in particular REG_DEAD
+ notes which are used by `avr.c::reg_unused_after' and branch offset
+ computations. These notes must be correct, i.e. there must be no
+ dangling REG_DEAD notes; otherwise wrong code might result, cf. PR64331.
+
+ DF needs (correct) CFG, hence right before free_cfg is the last
+ opportunity to rectify notes. */
+
+ register_pass (new avr_pass_recompute_notes (g, "avr-notes-free-cfg"),
+ PASS_POS_INSERT_BEFORE, "*free_cfg", 1);
+}
+
+
/* Implement `TARGET_OPTION_OVERRIDE'. */
static void
@@ -346,6 +400,11 @@ avr_option_override (void)
init_machine_status = avr_init_machine_status;
avr_log_set_avr_log();
+
+ /* Register some avr-specific pass(es). There is no canonical place for
+ pass registration. This function is convenient. */
+
+ avr_register_passes ();
}
/* Function to set up the backend function structure. */
Re: How to update reg_dead notes
it is a good policy to do a run with full checking every once in a
while. it is easy to have some garbage sneak in.df is particularly
sensitive to bad rtl so that was my first suggestion.i had forgotten
that df (as well as a lot of the infrastructure) does not work very late.
On 02/25/2015 01:54 PM, Georg-Johann Lay wrote:
Am 02/24/2015 um 07:33 PM schrieb Kenneth Zadeck:
when i suggested that you do a build with all of the checking turned
on, i
wanted you to this without you new code in it.there is a good
possibility
that the problem is that your port is generating bad rtl.
Ah, ok. This actually revealed an ice-checking; but that was not
related to the df problems...
I think the problem was that I tried to run passes needing df after
free_cfg.
My current solution works so far: It fixes the test case (correct code
+ insn lengths) and it did not ICE up to now.
Stuff is running a bit slow with all checks on so building and running
tests will take a while...
FYI, you find my current patch attached.
Thanks for your help and time,
Johann
Index: config/avr/avr.c
===
--- config/avr/avr.c(revision 220963)
+++ config/avr/avr.c(working copy)
@@ -51,6 +51,8 @@
#include "target-def.h"
#include "params.h"
#include "df.h"
+#include "context.h"
+#include "tree-pass.h"
/* Maximal allowed offset for an address in the LD command */
#define MAX_LD_OFFSET(MODE) (64 - (signed)GET_MODE_SIZE (MODE))
@@ -285,6 +287,58 @@ avr_to_int_mode (rtx x)
}
+static const pass_data avr_pass_data_recompute_notes =
+{
+ RTL_PASS, /* type */
+ "avr-xx", /* name (will be patched) */
+ OPTGROUP_NONE, /* optinfo_flags */
+ false, /* has_gate */
+ true, /* has_execute */
+ TV_DF_SCAN, /* tv_id */
+ 0, /* properties_required */
+ 0, /* properties_provided */
+ 0, /* properties_destroyed */
+ 0, /* todo_flags_start */
+ (TODO_df_finish | TODO_verify_rtl_sharing
+ | TODO_verify_flow ), /* todo_flags_finish */
+};
+
+
+class avr_pass_recompute_notes : public rtl_opt_pass
+{
+public:
+ avr_pass_recompute_notes (gcc::context *ctxt, const char *pass_name)
+: rtl_opt_pass (avr_pass_data_recompute_notes, ctxt)
+ {
+name = pass_name;
+ }
+
+ unsigned int execute ()
+ {
+df_note_add_problem ();
+df_analyze ();
+
+return 0;
+ }
+}; // avr_pass_recompute_notes
+
+
+static void
+avr_register_passes (void)
+{
+ /* This avr-specific pass (re)computes insn notes, in particular
REG_DEAD
+ notes which are used by `avr.c::reg_unused_after' and branch offset
+ computations. These notes must be correct, i.e. there must be no
+ dangling REG_DEAD notes; otherwise wrong code might result, cf.
PR64331.
+
+ DF needs (correct) CFG, hence right before free_cfg is the last
+ opportunity to rectify notes. */
+
+ register_pass (new avr_pass_recompute_notes (g, "avr-notes-free-cfg"),
+ PASS_POS_INSERT_BEFORE, "*free_cfg", 1);
+}
+
+
/* Implement `TARGET_OPTION_OVERRIDE'. */
static void
@@ -346,6 +400,11 @@ avr_option_override (void)
init_machine_status = avr_init_machine_status;
avr_log_set_avr_log();
+
+ /* Register some avr-specific pass(es). There is no canonical
place for
+ pass registration. This function is convenient. */
+
+ avr_register_passes ();
}
/* Function to set up the backend function structure. */
