* RTL sharing tester (for testing)
@ 2007-06-26 3:43 Jan Hubicka
2007-06-27 15:55 ` Kenneth Zadeck
2007-06-27 18:17 ` Diego Novillo
0 siblings, 2 replies; 28+ messages in thread
From: Jan Hubicka @ 2007-06-26 3:43 UTC (permalink / raw)
To: gcc-patches
Hi,
since dataflow code obviously is making RTL sharing issues worse, I
tried again the RTL sharing checker patch once again (attached in
updated form for mainline for everyone who wants to try and hunt down
some of the issues).
Without any patches I know of only ia-64 to bootstrap with one
regression in testusite (out of i686, x86-64 and PPC each failing for
different reasons), so the problem is still quite bad. It seems worse
than at early stage1 I tried last time despite the numberous sharing
fixes from dataflow branch, however I hope that number of different
issues is lesser so we can get it into shape that it does not break the
common targets used for development and thus can be enabled by default.
With the patch http://gcc.gnu.org/ml/gcc-patches/2007-06/msg01780.html
I can get i686 and x86-64 into bootstrapland with different 2 testsuite
regressions, both seems to be caused by regmove pass that is fixable by
the same infrastructure as introduced by the patch.
Any help with fixing the issues (or reviewing the aforementioned patch
;) would be very welcome.
Honza
Index: regrename.c
===================================================================
*** regrename.c (revision 125972)
--- regrename.c (working copy)
*************** struct tree_opt_pass pass_regrename =
*** 1960,1966 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func, /* todo_flags_finish */
'n' /* letter */
};
--- 1960,1966 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
'n' /* letter */
};
*************** struct tree_opt_pass pass_cprop_hardreg
*** 1993,1999 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func, /* todo_flags_finish */
'n' /* letter */
};
--- 1993,1999 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
'n' /* letter */
};
Index: fwprop.c
===================================================================
*** fwprop.c (revision 125972)
--- fwprop.c (working copy)
*************** struct tree_opt_pass pass_rtl_fwprop =
*** 1000,1006 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func, /* todo_flags_finish */
0 /* letter */
};
--- 1002,1008 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
0 /* letter */
};
*************** struct tree_opt_pass pass_rtl_fwprop_add
*** 1042,1048 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func, /* todo_flags_finish */
0 /* letter */
};
--- 1044,1050 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
0 /* letter */
};
Index: see.c
===================================================================
*** see.c (revision 125972)
--- see.c (working copy)
*************** struct tree_opt_pass pass_see =
*** 3837,3843 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func, /* todo_flags_finish */
'u' /* letter */
};
--- 3837,3843 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
'u' /* letter */
};
Index: tracer.c
===================================================================
*** tracer.c (revision 125972)
--- tracer.c (working copy)
*************** struct tree_opt_pass pass_tracer =
*** 411,417 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func, /* todo_flags_finish */
'T' /* letter */
};
--- 411,417 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
'T' /* letter */
};
Index: postreload-gcse.c
===================================================================
*** postreload-gcse.c (revision 125972)
--- postreload-gcse.c (working copy)
*************** struct tree_opt_pass pass_gcse2 =
*** 1322,1329 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func |
! TODO_verify_flow | TODO_ggc_collect, /* todo_flags_finish */
'J' /* letter */
};
--- 1322,1329 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing
! | TODO_verify_flow | TODO_ggc_collect,/* todo_flags_finish */
'J' /* letter */
};
Index: postreload.c
===================================================================
*** postreload.c (revision 125972)
--- postreload.c (working copy)
*************** struct tree_opt_pass pass_postreload_cse
*** 1599,1605 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func, /* todo_flags_finish */
'o' /* letter */
};
--- 1599,1605 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
'o' /* letter */
};
Index: tree-pass.h
===================================================================
*** tree-pass.h (revision 125972)
--- tree-pass.h (working copy)
*************** struct dump_file_info
*** 171,176 ****
--- 171,177 ----
#define TODO_dump_cgraph (1 << 7)
#define TODO_remove_functions (1 << 8)
#define TODO_rebuild_frequencies (1 << 9)
+ #define TODO_verify_rtl_sharing (1 << 10)
/* To-do flags for calls to update_ssa. */
*************** struct dump_file_info
*** 182,194 ****
in blocks that have one or more edges with no incoming definition
for O_j. This would lead to uninitialized warnings for O_j's
symbol. */
! #define TODO_update_ssa (1 << 10)
/* Update the SSA form without inserting any new PHI nodes at all.
This is used by passes that have either inserted all the PHI nodes
themselves or passes that need only to patch use-def and def-def
chains for virtuals (e.g., DCE). */
! #define TODO_update_ssa_no_phi (1 << 11)
/* Insert PHI nodes everywhere they are needed. No pruning of the
IDF is done. This is used by passes that need the PHI nodes for
--- 183,195 ----
in blocks that have one or more edges with no incoming definition
for O_j. This would lead to uninitialized warnings for O_j's
symbol. */
! #define TODO_update_ssa (1 << 11)
/* Update the SSA form without inserting any new PHI nodes at all.
This is used by passes that have either inserted all the PHI nodes
themselves or passes that need only to patch use-def and def-def
chains for virtuals (e.g., DCE). */
! #define TODO_update_ssa_no_phi (1 << 12)
/* Insert PHI nodes everywhere they are needed. No pruning of the
IDF is done. This is used by passes that need the PHI nodes for
*************** struct dump_file_info
*** 199,205 ****
may be doing something wrong. Inserting PHI nodes for an old name
where not all edges carry a new replacement may lead to silent
codegen errors or spurious uninitialized warnings. */
! #define TODO_update_ssa_full_phi (1 << 12)
/* Passes that update the SSA form on their own may want to delegate
the updating of virtual names to the generic updater. Since FUD
--- 200,206 ----
may be doing something wrong. Inserting PHI nodes for an old name
where not all edges carry a new replacement may lead to silent
codegen errors or spurious uninitialized warnings. */
! #define TODO_update_ssa_full_phi (1 << 13)
/* Passes that update the SSA form on their own may want to delegate
the updating of virtual names to the generic updater. Since FUD
*************** struct dump_file_info
*** 207,230 ****
to do. NOTE: If this flag is used, any OLD->NEW mappings for real
names are explicitly destroyed and only the symbols marked for
renaming are processed. */
! #define TODO_update_ssa_only_virtuals (1 << 13)
/* Some passes leave unused local variables that can be removed from
cfun->unexpanded_var_list. This reduces the size of dump files and
the memory footprint for VAR_DECLs. */
! #define TODO_remove_unused_locals (1 << 14)
/* Internally used for the first in a sequence of passes. It is set
for the passes that are handed to register_dump_files. */
! #define TODO_set_props (1 << 15)
/* Call df_finish at the end of the pass. This is done after all of
the dumpers have been allowed to run so that they have access to
the instance before it is destroyed. */
! #define TODO_df_finish (1 << 16)
/* Internally used for the first instance of a pass. */
! #define TODO_mark_first_instance (1 << 17)
#define TODO_update_ssa_any \
(TODO_update_ssa \
--- 208,231 ----
to do. NOTE: If this flag is used, any OLD->NEW mappings for real
names are explicitly destroyed and only the symbols marked for
renaming are processed. */
! #define TODO_update_ssa_only_virtuals (1 << 14)
/* Some passes leave unused local variables that can be removed from
cfun->unexpanded_var_list. This reduces the size of dump files and
the memory footprint for VAR_DECLs. */
! #define TODO_remove_unused_locals (1 << 15)
/* Internally used for the first in a sequence of passes. It is set
for the passes that are handed to register_dump_files. */
! #define TODO_set_props (1 << 16)
/* Call df_finish at the end of the pass. This is done after all of
the dumpers have been allowed to run so that they have access to
the instance before it is destroyed. */
! #define TODO_df_finish (1 << 17)
/* Internally used for the first instance of a pass. */
! #define TODO_mark_first_instance (1 << 18)
#define TODO_update_ssa_any \
(TODO_update_ssa \
Index: mode-switching.c
===================================================================
*** mode-switching.c (revision 125972)
--- mode-switching.c (working copy)
*************** struct tree_opt_pass pass_mode_switching
*** 745,751 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func, /* todo_flags_finish */
0 /* letter */
};
--- 745,751 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
0 /* letter */
};
Index: modulo-sched.c
===================================================================
*** modulo-sched.c (revision 125972)
--- modulo-sched.c (working copy)
*************** struct tree_opt_pass pass_sms =
*** 2502,2508 ****
0, /* properties_provided */
0, /* properties_destroyed */
TODO_dump_func, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'm' /* letter */
--- 2502,2508 ----
0, /* properties_provided */
0, /* properties_destroyed */
TODO_dump_func, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'm' /* letter */
Index: cse.c
===================================================================
*** cse.c (revision 125972)
--- cse.c (working copy)
*************** struct tree_opt_pass pass_cse =
*** 7012,7018 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func |
TODO_ggc_collect |
TODO_verify_flow, /* todo_flags_finish */
--- 7012,7018 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect |
TODO_verify_flow, /* todo_flags_finish */
*************** struct tree_opt_pass pass_cse2 =
*** 7070,7076 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func |
TODO_ggc_collect |
TODO_verify_flow, /* todo_flags_finish */
--- 7070,7076 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect |
TODO_verify_flow, /* todo_flags_finish */
Index: web.c
===================================================================
*** web.c (revision 125972)
--- web.c (working copy)
*************** struct tree_opt_pass pass_web =
*** 385,391 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func, /* todo_flags_finish */
'Z' /* letter */
};
--- 385,391 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
'Z' /* letter */
};
Index: loop-init.c
===================================================================
*** loop-init.c (revision 125972)
--- loop-init.c (working copy)
*************** struct tree_opt_pass pass_rtl_loop_init
*** 186,192 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func, /* todo_flags_finish */
'L' /* letter */
};
--- 186,192 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
'L' /* letter */
};
*************** struct tree_opt_pass pass_rtl_loop_done
*** 219,225 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func, /* todo_flags_finish */
'L' /* letter */
};
--- 219,225 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
'L' /* letter */
};
*************** struct tree_opt_pass pass_rtl_move_loop_
*** 252,258 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | /* This is shutting down the instance in loop_invariant.c */
TODO_dump_func, /* todo_flags_finish */
'L' /* letter */
};
--- 252,258 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing | /* This is shutting down the instance in loop_invariant.c */
TODO_dump_func, /* todo_flags_finish */
'L' /* letter */
};
*************** struct tree_opt_pass pass_rtl_unswitch =
*** 286,292 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func, /* todo_flags_finish */
'L' /* letter */
};
--- 286,292 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
'L' /* letter */
};
*************** struct tree_opt_pass pass_rtl_unroll_and
*** 332,338 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func, /* todo_flags_finish */
'L' /* letter */
};
--- 332,338 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
'L' /* letter */
};
*************** struct tree_opt_pass pass_rtl_doloop =
*** 371,377 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func, /* todo_flags_finish */
'L' /* letter */
};
--- 371,377 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
'L' /* letter */
};
Index: global.c
===================================================================
*** global.c (revision 125972)
--- global.c (working copy)
*************** struct tree_opt_pass pass_global_alloc =
*** 2109,2116 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func |
! TODO_ggc_collect, /* todo_flags_finish */
'g' /* letter */
};
--- 2109,2116 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing
! | TODO_ggc_collect, /* todo_flags_finish */
'g' /* letter */
};
Index: ifcvt.c
===================================================================
*** ifcvt.c (revision 125972)
--- ifcvt.c (working copy)
*************** struct tree_opt_pass pass_rtl_ifcvt =
*** 4088,4094 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func, /* todo_flags_finish */
'C' /* letter */
};
--- 4088,4094 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
'C' /* letter */
};
*************** struct tree_opt_pass pass_if_after_combi
*** 4124,4130 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'C' /* letter */
--- 4124,4130 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'C' /* letter */
*************** struct tree_opt_pass pass_if_after_reloa
*** 4158,4164 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'E' /* letter */
--- 4158,4164 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'E' /* letter */
Index: recog.c
===================================================================
*** recog.c (revision 125972)
--- recog.c (working copy)
*************** struct tree_opt_pass pass_peephole2 =
*** 3299,3305 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func, /* todo_flags_finish */
'z' /* letter */
};
--- 3299,3305 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
'z' /* letter */
};
*************** struct tree_opt_pass pass_split_for_shor
*** 3460,3466 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func, /* todo_flags_finish */
0 /* letter */
};
--- 3460,3466 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
0 /* letter */
};
Index: dse.c
===================================================================
*** dse.c (revision 125972)
--- dse.c (working copy)
*************** struct tree_opt_pass pass_rtl_dse1 =
*** 3082,3088 ****
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
! TODO_df_finish |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
--- 3082,3088 ----
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
*************** struct tree_opt_pass pass_rtl_dse2 =
*** 3101,3107 ****
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
! TODO_df_finish |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
--- 3101,3107 ----
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
Index: regmove.c
===================================================================
*** regmove.c (revision 125972)
--- regmove.c (working copy)
*************** struct tree_opt_pass pass_regmove =
*** 2122,2128 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'N' /* letter */
--- 2122,2128 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'N' /* letter */
Index: function.c
===================================================================
*** function.c (revision 125972)
--- function.c (working copy)
*************** struct tree_opt_pass pass_thread_prologu
*** 5500,5506 ****
0, /* properties_destroyed */
TODO_verify_flow, /* todo_flags_start */
TODO_dump_func |
! TODO_df_finish |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
--- 5500,5506 ----
0, /* properties_destroyed */
TODO_verify_flow, /* todo_flags_start */
TODO_dump_func |
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
Index: gcse.c
===================================================================
*** gcse.c (revision 125972)
--- gcse.c (working copy)
*************** struct tree_opt_pass pass_gcse =
*** 6742,6748 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func |
TODO_verify_flow | TODO_ggc_collect, /* todo_flags_finish */
'G' /* letter */
--- 6742,6748 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_verify_flow | TODO_ggc_collect, /* todo_flags_finish */
'G' /* letter */
Index: rtl-factoring.c
===================================================================
*** rtl-factoring.c (revision 125972)
--- rtl-factoring.c (working copy)
*************** struct tree_opt_pass pass_rtl_seqabstr =
*** 1427,1433 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'Q' /* letter */
--- 1427,1433 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'Q' /* letter */
Index: lower-subreg.c
===================================================================
*** lower-subreg.c (revision 125972)
--- lower-subreg.c (working copy)
*************** struct tree_opt_pass pass_lower_subreg2
*** 1293,1299 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func |
TODO_ggc_collect |
TODO_verify_flow, /* todo_flags_finish */
--- 1293,1299 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect |
TODO_verify_flow, /* todo_flags_finish */
Index: bt-load.c
===================================================================
*** bt-load.c (revision 125972)
--- bt-load.c (working copy)
*************** struct tree_opt_pass pass_branch_target_
*** 1520,1525 ****
--- 1520,1526 ----
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
+ TODO_verify_rtl_sharing |
TODO_ggc_collect, /* todo_flags_finish */
'd' /* letter */
};
Index: emit-rtl.c
===================================================================
*** emit-rtl.c (revision 125972)
--- emit-rtl.c (working copy)
*************** struct tree_opt_pass pass_unshare_all_rt
*** 2194,2200 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func, /* todo_flags_finish */
0 /* letter */
};
--- 2194,2200 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
0 /* letter */
};
Index: cfgcleanup.c
===================================================================
*** cfgcleanup.c (revision 125972)
--- cfgcleanup.c (working copy)
*************** struct tree_opt_pass pass_jump =
*** 2282,2288 ****
0, /* properties_provided */
0, /* properties_destroyed */
TODO_ggc_collect, /* todo_flags_start */
- TODO_dump_func |
TODO_verify_flow, /* todo_flags_finish */
'i' /* letter */
};
--- 2282,2287 ----
*************** struct tree_opt_pass pass_jump2 =
*** 2313,2319 ****
0, /* properties_provided */
0, /* properties_destroyed */
TODO_ggc_collect, /* todo_flags_start */
! TODO_dump_func, /* todo_flags_finish */
'j' /* letter */
};
--- 2312,2318 ----
0, /* properties_provided */
0, /* properties_destroyed */
TODO_ggc_collect, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing,/* todo_flags_finish */
'j' /* letter */
};
Index: combine.c
===================================================================
*** combine.c (revision 125972)
--- combine.c (working copy)
*************** struct tree_opt_pass pass_combine =
*** 12905,12911 ****
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
! TODO_df_finish |
TODO_ggc_collect, /* todo_flags_finish */
'c' /* letter */
};
--- 12905,12911 ----
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_ggc_collect, /* todo_flags_finish */
'c' /* letter */
};
Index: bb-reorder.c
===================================================================
*** bb-reorder.c (revision 125972)
--- bb-reorder.c (working copy)
*************** struct tree_opt_pass pass_duplicate_comp
*** 2085,2091 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func, /* todo_flags_finish */
0 /* letter */
};
--- 2085,2091 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing,/* todo_flags_finish */
0 /* letter */
};
*************** struct tree_opt_pass pass_reorder_blocks
*** 2235,2241 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func, /* todo_flags_finish */
'B' /* letter */
};
--- 2235,2241 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing,/* todo_flags_finish */
'B' /* letter */
};
*************** struct tree_opt_pass pass_partition_bloc
*** 2275,2281 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func, /* todo_flags_finish */
0 /* letter */
};
--- 2275,2281 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing,/* todo_flags_finish */
0 /* letter */
};
Index: var-tracking.c
===================================================================
*** var-tracking.c (revision 125972)
--- var-tracking.c (working copy)
*************** struct tree_opt_pass pass_variable_track
*** 3006,3012 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func, /* todo_flags_finish */
'V' /* letter */
};
--- 3006,3012 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_dump_func | TODO_verify_rtl_sharing,/* todo_flags_finish */
'V' /* letter */
};
Index: reg-stack.c
===================================================================
*** reg-stack.c (revision 125972)
--- reg-stack.c (working copy)
*************** struct tree_opt_pass pass_stack_regs_run
*** 3243,3249 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'k' /* letter */
--- 3243,3249 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'k' /* letter */
Index: sched-rgn.c
===================================================================
*** sched-rgn.c (revision 125972)
--- sched-rgn.c (working copy)
*************** struct tree_opt_pass pass_sched =
*** 3169,3175 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func |
TODO_verify_flow |
TODO_ggc_collect, /* todo_flags_finish */
--- 3169,3175 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_verify_flow |
TODO_ggc_collect, /* todo_flags_finish */
*************** struct tree_opt_pass pass_sched2 =
*** 3189,3195 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func |
TODO_verify_flow |
TODO_ggc_collect, /* todo_flags_finish */
--- 3189,3195 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_verify_flow |
TODO_ggc_collect, /* todo_flags_finish */
Index: passes.c
===================================================================
*** passes.c (revision 125972)
--- passes.c (working copy)
*************** struct tree_opt_pass pass_postreload =
*** 321,327 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_ggc_collect, /* todo_flags_finish */
0 /* letter */
};
--- 321,327 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_ggc_collect | TODO_verify_rtl_sharing, /* todo_flags_finish */
0 /* letter */
};
*************** execute_function_todo (void *data)
*** 976,981 ****
--- 976,983 ----
verify_stmts ();
if (flags & TODO_verify_loops)
verify_loop_closed_ssa ();
+ if (flags & TODO_verify_rtl_sharing)
+ verify_rtl_sharing ();
#endif
cfun->last_verified = flags & TODO_verify_all;
Index: combine-stack-adj.c
===================================================================
*** combine-stack-adj.c (revision 125972)
--- combine-stack-adj.c (working copy)
*************** struct tree_opt_pass pass_stack_adjustme
*** 486,492 ****
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
0 /* letter */
--- 486,492 ----
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
0 /* letter */
Index: dce.c
===================================================================
*** dce.c (revision 125972)
--- dce.c (working copy)
*************** struct tree_opt_pass pass_ud_rtl_dce =
*** 483,489 ****
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
! TODO_df_finish |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
--- 483,489 ----
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
*************** struct tree_opt_pass pass_fast_rtl_dce =
*** 790,796 ****
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
! TODO_df_finish |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
--- 790,796 ----
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
! TODO_df_finish | TODO_verify_rtl_sharing |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: RTL sharing tester (for testing)
2007-06-26 3:43 RTL sharing tester (for testing) Jan Hubicka
@ 2007-06-27 15:55 ` Kenneth Zadeck
2007-06-27 18:17 ` Diego Novillo
1 sibling, 0 replies; 28+ messages in thread
From: Kenneth Zadeck @ 2007-06-27 15:55 UTC (permalink / raw)
To: gcc-patches; +Cc: Jan Hubicka
> Hi,
> since dataflow code obviously is making RTL sharing issues worse, I
> tried again the RTL sharing checker patch once again (attached in
> updated form for mainline for everyone who wants to try and hunt down
> some of the issues).
>
> Without any patches I know of only ia-64 to bootstrap with one
> regression in testusite (out of i686, x86-64 and PPC each failing for
> different reasons), so the problem is still quite bad. It seems worse
> than at early stage1 I tried last time despite the numberous sharing
> fixes from dataflow branch, however I hope that number of different
> issues is lesser so we can get it into shape that it does not break the
> common targets used for development and thus can be enabled by default.
>
> With the patch http://gcc.gnu.org/ml/gcc-patches/2007-06/msg01780.html
> I can get i686 and x86-64 into bootstrapland with different 2 testsuite
> regressions, both seems to be caused by regmove pass that is fixable by
> the same infrastructure as introduced by the patch.
>
> Any help with fixing the issues (or reviewing the aforementioned patch
> ;) would be very welcome.
>
> Honza
Honza,
this is a lost cause unless you get permission to actually commit this
patch under stage II and get people to just buy into the grief. You
will never be able to completely catch up, unless people IMMEDIATELY
and ALWAYS see the error of their ways.
What i would suggest in the short term, is for you to hack this patch
so that the checking is only enabled if you configure with some
special --enable-checking option. That at least lowers the cost of
checking and is something that could be checked in with little
controversy.
However, until you get permission to make this patch the default for
--enable-checking, you will be fighting a very labor intensive loosing
battle.
I will say that having chased down too many obscure bugs caused by
sharing, (because as you point out, df is more sensitive to this
problem) I am strongly in favor of this patch.
kenny
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-26 3:43 RTL sharing tester (for testing) Jan Hubicka
2007-06-27 15:55 ` Kenneth Zadeck
@ 2007-06-27 18:17 ` Diego Novillo
2007-06-27 21:43 ` Jan Hubicka
2007-06-27 23:06 ` Mark Mitchell
1 sibling, 2 replies; 28+ messages in thread
From: Diego Novillo @ 2007-06-27 18:17 UTC (permalink / raw)
To: Jan Hubicka; +Cc: gcc-patches, Mark Mitchell
On 6/25/07 8:01 PM, Jan Hubicka wrote:
> With the patch http://gcc.gnu.org/ml/gcc-patches/2007-06/msg01780.html
> I can get i686 and x86-64 into bootstrapland with different 2 testsuite
> regressions, both seems to be caused by regmove pass that is fixable by
> the same infrastructure as introduced by the patch.
How expensive is this verification? My inclination is to enable the
sharing verifier in a couple of steps:
1- Right now, enable it in every pass that doesn't expose bootstrap
problems. I'm actually willing to live with breakage in the testsuite
and address it during stages 2 and 3.
2- Once the bootstrap problems are fixed, leave the verifier enabled
everywhere.
But since this will affect stages 2 and 3, I'd like to get Mark's opinion.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-27 18:17 ` Diego Novillo
@ 2007-06-27 21:43 ` Jan Hubicka
2007-06-28 2:02 ` Kaz Kojima
2007-06-27 23:06 ` Mark Mitchell
1 sibling, 1 reply; 28+ messages in thread
From: Jan Hubicka @ 2007-06-27 21:43 UTC (permalink / raw)
To: Diego Novillo; +Cc: Jan Hubicka, gcc-patches, Mark Mitchell
> On 6/25/07 8:01 PM, Jan Hubicka wrote:
>
> > With the patch http://gcc.gnu.org/ml/gcc-patches/2007-06/msg01780.html
> > I can get i686 and x86-64 into bootstrapland with different 2 testsuite
> > regressions, both seems to be caused by regmove pass that is fixable by
> > the same infrastructure as introduced by the patch.
>
>
> How expensive is this verification? My inclination is to enable the
Not overly expensive, it is simple pass over RTL chain setting/clearing
the flags. I can get more precise data, but I had timevar for it and
never saw really important percentages (unlike SSA verifier for
instance)
I am just running another round of tests on the mainline after the
fixes. Now i686 is clean, x86-64 didn't complete yet (last time there
was one testsuite regression I have patch for in queue), ia64 has one
failure same as x86-64 had:
gcc.c-torture/execute/loop-3b.c compilation, -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions (internal compiler error)
gcc.c-torture/execute/loop-3b.c compilation, -O3 -fomit-frame-pointer -funroll-loops (internal compiler error)
PPC fails on:
/abuild/gcc-test/gcc/libjava/classpath/native/fdlibm/s_expm1.c: In function 'expm1':
/abuild/gcc-test/gcc/libjava/classpath/native/fdlibm/s_expm1.c:228: error: invalid rtl sharing found in the insn
(insn:TI 331 524 604 36 /abuild/gcc-test/gcc/libjava/classpath/native/fdlibm/s_expm1.c:207 (set (reg/v:DF 33 1 [orig:161 x ] [161])
(minus:DF (reg:DF 32 0)
(reg:DF 39 7 [251]))) 200 {*subdf3_fpr} (expr_list:REG_DEAD (reg:DF 39 7 [251])
(expr_list:REG_DEAD (reg:DF 32 0)
(expr_list:REG_DEAD (subreg:DF (reg/v:DI 9 9 [orig:153 sh_u ] [153]) 0)
(expr_list:REG_EQUAL (minus:DF (subreg:DF (reg/v:DI 9 9 [orig:153 sh_u ] [153]) 0)
(const_double:DF 1.0e+0 [0x0.8p+1]))
(nil))))))
/abuild/gcc-test/gcc/libjava/classpath/native/fdlibm/s_expm1.c:228: error: shared rtx
(subreg:DF (reg/v:DI 9 9 [orig:153 sh_u ] [153]) 0)
/abuild/gcc-test/gcc/libjava/classpath/native/fdlibm/s_expm1.c:228: internal compiler error: internal consistency failure
It bootstraps without Java.
PPC64 didn't complette yet.
> sharing verifier in a couple of steps:
>
> 1- Right now, enable it in every pass that doesn't expose bootstrap
> problems. I'm actually willing to live with breakage in the testsuite
> and address it during stages 2 and 3.
The problem with is that the nature of problem is very target specific.
All the remaining problems are either very weird side cases, or target
issues. This is why I am posting the patch so people can test it on
their favorite development platform.
My plan is to debug the PPC failure so all testing targets we have
(i686/x86-64/ia64/ppc/ppc64 are known to bootstrap) and then propose it
for enabling by default.
It is however likely that other targets will expose new problems. Mips
was once brought into bootstrapland in early stage1.
>
> 2- Once the bootstrap problems are fixed, leave the verifier enabled
> everywhere.
>
> But since this will affect stages 2 and 3, I'd like to get Mark's opinion.
Honza
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-27 18:17 ` Diego Novillo
2007-06-27 21:43 ` Jan Hubicka
@ 2007-06-27 23:06 ` Mark Mitchell
2007-06-27 23:06 ` Jan Hubicka
1 sibling, 1 reply; 28+ messages in thread
From: Mark Mitchell @ 2007-06-27 23:06 UTC (permalink / raw)
To: Diego Novillo; +Cc: Jan Hubicka, gcc-patches
Diego Novillo wrote:
> How expensive is this verification? My inclination is to enable the
> sharing verifier in a couple of steps:
>
> 1- Right now, enable it in every pass that doesn't expose bootstrap
> problems. I'm actually willing to live with breakage in the testsuite
> and address it during stages 2 and 3.
>
> 2- Once the bootstrap problems are fixed, leave the verifier enabled
> everywhere.
>
> But since this will affect stages 2 and 3, I'd like to get Mark's opinion.
I'd like to turn it on only where it does not cause breakage in either
bootstrapping or testing. Maintaining stability of the testsuite
results, so that we can tell what's a regression and what isn't, is very
useful. Let's at least get it to the point where it's clean on the
primary platforms before turning it on by default.
Thanks,
--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-27 23:06 ` Mark Mitchell
@ 2007-06-27 23:06 ` Jan Hubicka
2007-06-27 23:12 ` Eric Christopher
0 siblings, 1 reply; 28+ messages in thread
From: Jan Hubicka @ 2007-06-27 23:06 UTC (permalink / raw)
To: Mark Mitchell; +Cc: Diego Novillo, Jan Hubicka, gcc-patches
> Diego Novillo wrote:
>
> > How expensive is this verification? My inclination is to enable the
> > sharing verifier in a couple of steps:
> >
> > 1- Right now, enable it in every pass that doesn't expose bootstrap
> > problems. I'm actually willing to live with breakage in the testsuite
> > and address it during stages 2 and 3.
> >
> > 2- Once the bootstrap problems are fixed, leave the verifier enabled
> > everywhere.
> >
> > But since this will affect stages 2 and 3, I'd like to get Mark's opinion.
>
> I'd like to turn it on only where it does not cause breakage in either
> bootstrapping or testing. Maintaining stability of the testsuite
> results, so that we can tell what's a regression and what isn't, is very
> useful. Let's at least get it to the point where it's clean on the
> primary platforms before turning it on by defaultA
OK, from the list of primary platforms, I have access to:
* i686-pc-linux-gnu
* x86_64-unknown-linux-gnu
* powerpc64-unknown-linux-gnu
I know powerpc64 breaks in libjava what I plan to fix tomorrow.
Would be possible to test the patch on the remaining platforms and help
me to debug it, please?
* arm-eabi
* i386-unknown-freebsd
* i686-apple-darwin
* mipsisa64-elf
* sparc-sun-solaris2.10
Honza
>
> Thanks,
>
> --
> Mark Mitchell
> CodeSourcery
> mark@codesourcery.com
> (650) 331-3385 x713
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-27 23:06 ` Jan Hubicka
@ 2007-06-27 23:12 ` Eric Christopher
2007-06-28 0:48 ` Jan Hubicka
0 siblings, 1 reply; 28+ messages in thread
From: Eric Christopher @ 2007-06-27 23:12 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Mark Mitchell, Diego Novillo, Jan Hubicka, gcc-patches
>
> OK, from the list of primary platforms, I have access to:
>
> * i686-pc-linux-gnu
> * x86_64-unknown-linux-gnu
> * powerpc64-unknown-linux-gnu
>
> I know powerpc64 breaks in libjava what I plan to fix tomorrow.
> Would be possible to test the patch on the remaining platforms and
> help
> me to debug it, please?
>
> * i686-apple-darwin
> * mipsisa64-elf
I can help on these two sure. Especially since I'm in favor of the
patch :)
Jan: Can you send it to me please?
-eric
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-27 23:12 ` Eric Christopher
@ 2007-06-28 0:48 ` Jan Hubicka
2007-06-28 22:24 ` Eric Christopher
2007-06-29 23:14 ` Graham Stott
0 siblings, 2 replies; 28+ messages in thread
From: Jan Hubicka @ 2007-06-28 0:48 UTC (permalink / raw)
To: Eric Christopher
Cc: Jan Hubicka, Mark Mitchell, Diego Novillo, Jan Hubicka, gcc-patches
> >
> >OK, from the list of primary platforms, I have access to:
> >
> > * i686-pc-linux-gnu
> > * x86_64-unknown-linux-gnu
> > * powerpc64-unknown-linux-gnu
> >
> >I know powerpc64 breaks in libjava what I plan to fix tomorrow.
> >Would be possible to test the patch on the remaining platforms and
> >help
> >me to debug it, please?
> >
> > * i686-apple-darwin
> > * mipsisa64-elf
>
> I can help on these two sure. Especially since I'm in favor of the
> patch :)
>
> Jan: Can you send it to me please?
Sure, I am attaching version I currently use for testing.
Thanks a lot! (with a bit of luck i686 at least should go smoothly)
Honza
Index: regrename.c
===================================================================
--- regrename.c (revision 125970)
+++ regrename.c (working copy)
@@ -1960,7 +1960,7 @@ struct tree_opt_pass pass_regrename =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
'n' /* letter */
};
@@ -1993,7 +1993,7 @@ struct tree_opt_pass pass_cprop_hardreg
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
'n' /* letter */
};
Index: fwprop.c
===================================================================
--- fwprop.c (revision 125970)
+++ fwprop.c (working copy)
@@ -1000,7 +1000,7 @@ struct tree_opt_pass pass_rtl_fwprop =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
0 /* letter */
};
@@ -1042,7 +1042,7 @@ struct tree_opt_pass pass_rtl_fwprop_add
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
0 /* letter */
};
Index: see.c
===================================================================
--- see.c (revision 125970)
+++ see.c (working copy)
@@ -3837,7 +3837,7 @@ struct tree_opt_pass pass_see =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
'u' /* letter */
};
Index: tracer.c
===================================================================
--- tracer.c (revision 125970)
+++ tracer.c (working copy)
@@ -411,7 +411,7 @@ struct tree_opt_pass pass_tracer =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
'T' /* letter */
};
Index: postreload-gcse.c
===================================================================
--- postreload-gcse.c (revision 125970)
+++ postreload-gcse.c (working copy)
@@ -1322,8 +1322,8 @@ struct tree_opt_pass pass_gcse2 =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func |
- TODO_verify_flow | TODO_ggc_collect, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing
+ | TODO_verify_flow | TODO_ggc_collect,/* todo_flags_finish */
'J' /* letter */
};
Index: postreload.c
===================================================================
--- postreload.c (revision 125970)
+++ postreload.c (working copy)
@@ -1599,7 +1599,7 @@ struct tree_opt_pass pass_postreload_cse
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
'o' /* letter */
};
Index: tree-pass.h
===================================================================
--- tree-pass.h (revision 125970)
+++ tree-pass.h (working copy)
@@ -171,6 +171,7 @@ struct dump_file_info
#define TODO_dump_cgraph (1 << 7)
#define TODO_remove_functions (1 << 8)
#define TODO_rebuild_frequencies (1 << 9)
+#define TODO_verify_rtl_sharing (1 << 10)
/* To-do flags for calls to update_ssa. */
@@ -182,13 +183,13 @@ struct dump_file_info
in blocks that have one or more edges with no incoming definition
for O_j. This would lead to uninitialized warnings for O_j's
symbol. */
-#define TODO_update_ssa (1 << 10)
+#define TODO_update_ssa (1 << 11)
/* Update the SSA form without inserting any new PHI nodes at all.
This is used by passes that have either inserted all the PHI nodes
themselves or passes that need only to patch use-def and def-def
chains for virtuals (e.g., DCE). */
-#define TODO_update_ssa_no_phi (1 << 11)
+#define TODO_update_ssa_no_phi (1 << 12)
/* Insert PHI nodes everywhere they are needed. No pruning of the
IDF is done. This is used by passes that need the PHI nodes for
@@ -199,7 +200,7 @@ struct dump_file_info
may be doing something wrong. Inserting PHI nodes for an old name
where not all edges carry a new replacement may lead to silent
codegen errors or spurious uninitialized warnings. */
-#define TODO_update_ssa_full_phi (1 << 12)
+#define TODO_update_ssa_full_phi (1 << 13)
/* Passes that update the SSA form on their own may want to delegate
the updating of virtual names to the generic updater. Since FUD
@@ -207,24 +208,24 @@ struct dump_file_info
to do. NOTE: If this flag is used, any OLD->NEW mappings for real
names are explicitly destroyed and only the symbols marked for
renaming are processed. */
-#define TODO_update_ssa_only_virtuals (1 << 13)
+#define TODO_update_ssa_only_virtuals (1 << 14)
/* Some passes leave unused local variables that can be removed from
cfun->unexpanded_var_list. This reduces the size of dump files and
the memory footprint for VAR_DECLs. */
-#define TODO_remove_unused_locals (1 << 14)
+#define TODO_remove_unused_locals (1 << 15)
/* Internally used for the first in a sequence of passes. It is set
for the passes that are handed to register_dump_files. */
-#define TODO_set_props (1 << 15)
+#define TODO_set_props (1 << 16)
/* Call df_finish at the end of the pass. This is done after all of
the dumpers have been allowed to run so that they have access to
the instance before it is destroyed. */
-#define TODO_df_finish (1 << 16)
+#define TODO_df_finish (1 << 17)
/* Internally used for the first instance of a pass. */
-#define TODO_mark_first_instance (1 << 17)
+#define TODO_mark_first_instance (1 << 18)
#define TODO_update_ssa_any \
(TODO_update_ssa \
Index: mode-switching.c
===================================================================
--- mode-switching.c (revision 125970)
+++ mode-switching.c (working copy)
@@ -745,7 +745,7 @@ struct tree_opt_pass pass_mode_switching
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
0 /* letter */
};
Index: modulo-sched.c
===================================================================
--- modulo-sched.c (revision 125970)
+++ modulo-sched.c (working copy)
@@ -2502,7 +2502,7 @@ struct tree_opt_pass pass_sms =
0, /* properties_provided */
0, /* properties_destroyed */
TODO_dump_func, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'm' /* letter */
Index: cse.c
===================================================================
--- cse.c (revision 125970)
+++ cse.c (working copy)
@@ -7012,7 +7012,7 @@ struct tree_opt_pass pass_cse =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect |
TODO_verify_flow, /* todo_flags_finish */
@@ -7070,7 +7070,7 @@ struct tree_opt_pass pass_cse2 =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect |
TODO_verify_flow, /* todo_flags_finish */
Index: web.c
===================================================================
--- web.c (revision 125970)
+++ web.c (working copy)
@@ -385,7 +385,7 @@ struct tree_opt_pass pass_web =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
'Z' /* letter */
};
Index: loop-init.c
===================================================================
--- loop-init.c (revision 125970)
+++ loop-init.c (working copy)
@@ -186,7 +186,7 @@ struct tree_opt_pass pass_rtl_loop_init
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
'L' /* letter */
};
@@ -219,7 +219,7 @@ struct tree_opt_pass pass_rtl_loop_done
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
'L' /* letter */
};
@@ -252,7 +252,7 @@ struct tree_opt_pass pass_rtl_move_loop_
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish | /* This is shutting down the instance in loop_invariant.c */
+ TODO_df_finish | TODO_verify_rtl_sharing | /* This is shutting down the instance in loop_invariant.c */
TODO_dump_func, /* todo_flags_finish */
'L' /* letter */
};
@@ -286,7 +286,7 @@ struct tree_opt_pass pass_rtl_unswitch =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
'L' /* letter */
};
@@ -332,7 +332,7 @@ struct tree_opt_pass pass_rtl_unroll_and
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
'L' /* letter */
};
@@ -371,7 +371,7 @@ struct tree_opt_pass pass_rtl_doloop =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
'L' /* letter */
};
Index: global.c
===================================================================
--- global.c (revision 125970)
+++ global.c (working copy)
@@ -2109,8 +2109,8 @@ struct tree_opt_pass pass_global_alloc =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func |
- TODO_ggc_collect, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing
+ | TODO_ggc_collect, /* todo_flags_finish */
'g' /* letter */
};
Index: ifcvt.c
===================================================================
--- ifcvt.c (revision 125970)
+++ ifcvt.c (working copy)
@@ -4088,7 +4088,7 @@ struct tree_opt_pass pass_rtl_ifcvt =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
'C' /* letter */
};
@@ -4124,7 +4124,7 @@ struct tree_opt_pass pass_if_after_combi
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'C' /* letter */
@@ -4158,7 +4158,7 @@ struct tree_opt_pass pass_if_after_reloa
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'E' /* letter */
Index: recog.c
===================================================================
--- recog.c (revision 125970)
+++ recog.c (working copy)
@@ -3299,7 +3299,7 @@ struct tree_opt_pass pass_peephole2 =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func, /* todo_flags_finish */
'z' /* letter */
};
@@ -3460,7 +3460,7 @@ struct tree_opt_pass pass_split_for_shor
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
0 /* letter */
};
Index: dse.c
===================================================================
--- dse.c (revision 125970)
+++ dse.c (working copy)
@@ -3082,7 +3082,7 @@ struct tree_opt_pass pass_rtl_dse1 =
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
@@ -3101,7 +3101,7 @@ struct tree_opt_pass pass_rtl_dse2 =
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
Index: regmove.c
===================================================================
--- regmove.c (revision 125970)
+++ regmove.c (working copy)
@@ -2122,7 +2122,7 @@ struct tree_opt_pass pass_regmove =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'N' /* letter */
Index: function.c
===================================================================
--- function.c (revision 125970)
+++ function.c (working copy)
@@ -5500,7 +5500,7 @@ struct tree_opt_pass pass_thread_prologu
0, /* properties_destroyed */
TODO_verify_flow, /* todo_flags_start */
TODO_dump_func |
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
Index: gcse.c
===================================================================
--- gcse.c (revision 125970)
+++ gcse.c (working copy)
@@ -6742,7 +6742,7 @@ struct tree_opt_pass pass_gcse =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_verify_flow | TODO_ggc_collect, /* todo_flags_finish */
'G' /* letter */
Index: rtl-factoring.c
===================================================================
--- rtl-factoring.c (revision 125970)
+++ rtl-factoring.c (working copy)
@@ -1427,7 +1427,7 @@ struct tree_opt_pass pass_rtl_seqabstr =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'Q' /* letter */
Index: lower-subreg.c
===================================================================
--- lower-subreg.c (revision 125970)
+++ lower-subreg.c (working copy)
@@ -1293,7 +1293,7 @@ struct tree_opt_pass pass_lower_subreg2
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect |
TODO_verify_flow, /* todo_flags_finish */
Index: bt-load.c
===================================================================
--- bt-load.c (revision 125970)
+++ bt-load.c (working copy)
@@ -1520,6 +1520,7 @@ struct tree_opt_pass pass_branch_target_
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
+ TODO_verify_rtl_sharing |
TODO_ggc_collect, /* todo_flags_finish */
'd' /* letter */
};
Index: emit-rtl.c
===================================================================
--- emit-rtl.c (revision 125970)
+++ emit-rtl.c (working copy)
@@ -2194,7 +2194,7 @@ struct tree_opt_pass pass_unshare_all_rt
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing, /* todo_flags_finish */
0 /* letter */
};
Index: cfgcleanup.c
===================================================================
--- cfgcleanup.c (revision 125970)
+++ cfgcleanup.c (working copy)
@@ -2282,7 +2282,6 @@ struct tree_opt_pass pass_jump =
0, /* properties_provided */
0, /* properties_destroyed */
TODO_ggc_collect, /* todo_flags_start */
- TODO_dump_func |
TODO_verify_flow, /* todo_flags_finish */
'i' /* letter */
};
@@ -2313,7 +2312,7 @@ struct tree_opt_pass pass_jump2 =
0, /* properties_provided */
0, /* properties_destroyed */
TODO_ggc_collect, /* todo_flags_start */
- TODO_dump_func, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing,/* todo_flags_finish */
'j' /* letter */
};
Index: combine.c
===================================================================
--- combine.c (revision 125970)
+++ combine.c (working copy)
@@ -12905,7 +12905,7 @@ struct tree_opt_pass pass_combine =
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_ggc_collect, /* todo_flags_finish */
'c' /* letter */
};
Index: bb-reorder.c
===================================================================
--- bb-reorder.c (revision 125970)
+++ bb-reorder.c (working copy)
@@ -2085,7 +2085,7 @@ struct tree_opt_pass pass_duplicate_comp
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing,/* todo_flags_finish */
0 /* letter */
};
@@ -2235,7 +2235,7 @@ struct tree_opt_pass pass_reorder_blocks
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing,/* todo_flags_finish */
'B' /* letter */
};
@@ -2275,7 +2275,7 @@ struct tree_opt_pass pass_partition_bloc
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing,/* todo_flags_finish */
0 /* letter */
};
Index: var-tracking.c
===================================================================
--- var-tracking.c (revision 125970)
+++ var-tracking.c (working copy)
@@ -3006,7 +3006,7 @@ struct tree_opt_pass pass_variable_track
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_dump_func, /* todo_flags_finish */
+ TODO_dump_func | TODO_verify_rtl_sharing,/* todo_flags_finish */
'V' /* letter */
};
Index: reg-stack.c
===================================================================
--- reg-stack.c (revision 125970)
+++ reg-stack.c (working copy)
@@ -3243,7 +3243,7 @@ struct tree_opt_pass pass_stack_regs_run
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
'k' /* letter */
Index: sched-rgn.c
===================================================================
--- sched-rgn.c (revision 125970)
+++ sched-rgn.c (working copy)
@@ -3169,7 +3169,7 @@ struct tree_opt_pass pass_sched =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_verify_flow |
TODO_ggc_collect, /* todo_flags_finish */
@@ -3189,7 +3189,7 @@ struct tree_opt_pass pass_sched2 =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_verify_flow |
TODO_ggc_collect, /* todo_flags_finish */
Index: passes.c
===================================================================
--- passes.c (revision 125970)
+++ passes.c (working copy)
@@ -321,7 +321,7 @@ struct tree_opt_pass pass_postreload =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_ggc_collect, /* todo_flags_finish */
+ TODO_ggc_collect | TODO_verify_rtl_sharing, /* todo_flags_finish */
0 /* letter */
};
@@ -976,6 +976,8 @@ execute_function_todo (void *data)
verify_stmts ();
if (flags & TODO_verify_loops)
verify_loop_closed_ssa ();
+ if (flags & TODO_verify_rtl_sharing)
+ verify_rtl_sharing ();
#endif
cfun->last_verified = flags & TODO_verify_all;
Index: combine-stack-adj.c
===================================================================
--- combine-stack-adj.c (revision 125970)
+++ combine-stack-adj.c (working copy)
@@ -486,7 +486,7 @@ struct tree_opt_pass pass_stack_adjustme
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_dump_func |
TODO_ggc_collect, /* todo_flags_finish */
0 /* letter */
Index: dce.c
===================================================================
--- dce.c (revision 125970)
+++ dce.c (working copy)
@@ -475,7 +475,7 @@ struct tree_opt_pass pass_ud_rtl_dce =
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
@@ -782,7 +782,7 @@ struct tree_opt_pass pass_fast_rtl_dce =
0, /* properties_destroyed */
0, /* todo_flags_start */
TODO_dump_func |
- TODO_df_finish |
+ TODO_df_finish | TODO_verify_rtl_sharing |
TODO_ggc_collect, /* todo_flags_finish */
'w' /* letter */
};
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-27 21:43 ` Jan Hubicka
@ 2007-06-28 2:02 ` Kaz Kojima
2007-06-28 4:24 ` Jan Hubicka
0 siblings, 1 reply; 28+ messages in thread
From: Kaz Kojima @ 2007-06-28 2:02 UTC (permalink / raw)
To: jh; +Cc: gcc-patches
Jan Hubicka <jh@suse.cz> wrote:
> The problem with is that the nature of problem is very target specific.
> All the remaining problems are either very weird side cases, or target
> issues. This is why I am posting the patch so people can test it on
> their favorite development platform.
FYI, I've tried your tester on SH and got an error which looks
similar to PPC's one:
../../../../../../TMP/trunk/libjava/classpath/native/fdlibm/e_exp.c:164: error: invalid rtl sharing found in the insn
(insn:HI 207 303 256 24 ../../../../../../TMP/trunk/libjava/classpath/native/fdlibm/e_exp.c:162 (parallel [
(set (reg/v:DF 76 fr12 [orig:180 x ] [180])
(mult:DF (reg/v:DF 76 fr12 [orig:180 x ] [180])
(reg:DF 66 fr2)))
(use (reg/v:PSI 151 ))
]) 340 {muldf3_i} (expr_list:REG_DEAD (reg/v:PSI 151 )
(expr_list:REG_DEAD (subreg:DF (reg/v:DI 1 r1 [orig:178 sh_u ] [178]) 0) (expr_list:REG_DEAD (reg:DF 66 fr2)
(expr_list:REG_EQUAL (mult:DF (subreg:DF (reg/v:DI 1 r1 [orig:178 sh_u ] [178]) 0)
(const_double:DF 9.33263618503218878990089544723817169617091446372e-302 [0x0.8p-999]))
(nil))))))
../../../../../../TMP/trunk/libjava/classpath/native/fdlibm/e_exp.c:164: error: shared rtx
(subreg:DF (reg/v:DI 1 r1 [orig:178 sh_u ] [178]) 0)
../../../../../../TMP/trunk/libjava/classpath/native/fdlibm/e_exp.c:164: internal compiler error: internal consistency failure
and SH bootstraps without Java. As an experiment, I've tried a one
liner:
--- ORIG/trunk/gcc/df-problems.c 2007-06-22 09:16:00.000000000 +0900
+++ TMP/trunk/gcc/df-problems.c 2007-06-26 13:53:47.000000000 +0900
@@ -3713,7 +3713,8 @@ df_set_note (enum reg_note note_type, rt
}
/* Did not find the note. */
- REG_NOTES (insn) = alloc_EXPR_LIST (note_type, reg, REG_NOTES (insn));
+ REG_NOTES (insn) = alloc_EXPR_LIST (note_type, copy_rtx (reg),
+ REG_NOTES (insn));
return old;
}
Then the above error on SH went away. This one liner may be
an overkill, but I hope that this experiment will help experts
to narrow down the problem.
Regards,
kaz
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-28 2:02 ` Kaz Kojima
@ 2007-06-28 4:24 ` Jan Hubicka
2007-06-28 8:11 ` Paolo Bonzini
2007-06-28 17:16 ` Kenneth Zadeck
0 siblings, 2 replies; 28+ messages in thread
From: Jan Hubicka @ 2007-06-28 4:24 UTC (permalink / raw)
To: Kaz Kojima; +Cc: jh, gcc-patches
> Jan Hubicka <jh@suse.cz> wrote:
> > The problem with is that the nature of problem is very target specific.
> > All the remaining problems are either very weird side cases, or target
> > issues. This is why I am posting the patch so people can test it on
> > their favorite development platform.
>
> FYI, I've tried your tester on SH and got an error which looks
> similar to PPC's one:
>
> ../../../../../../TMP/trunk/libjava/classpath/native/fdlibm/e_exp.c:164: error: invalid rtl sharing found in the insn
> (insn:HI 207 303 256 24 ../../../../../../TMP/trunk/libjava/classpath/native/fdlibm/e_exp.c:162 (parallel [
> (set (reg/v:DF 76 fr12 [orig:180 x ] [180])
> (mult:DF (reg/v:DF 76 fr12 [orig:180 x ] [180])
> (reg:DF 66 fr2)))
> (use (reg/v:PSI 151 ))
> ]) 340 {muldf3_i} (expr_list:REG_DEAD (reg/v:PSI 151 )
> (expr_list:REG_DEAD (subreg:DF (reg/v:DI 1 r1 [orig:178 sh_u ] [178]) 0) (expr_list:REG_DEAD (reg:DF 66 fr2)
> (expr_list:REG_EQUAL (mult:DF (subreg:DF (reg/v:DI 1 r1 [orig:178 sh_u ] [178]) 0)
> (const_double:DF 9.33263618503218878990089544723817169617091446372e-302 [0x0.8p-999]))
> (nil))))))
> ../../../../../../TMP/trunk/libjava/classpath/native/fdlibm/e_exp.c:164: error: shared rtx
> (subreg:DF (reg/v:DI 1 r1 [orig:178 sh_u ] [178]) 0)
> ../../../../../../TMP/trunk/libjava/classpath/native/fdlibm/e_exp.c:164: internal compiler error: internal consistency failure
>
> and SH bootstraps without Java. As an experiment, I've tried a one
> liner:
>
> --- ORIG/trunk/gcc/df-problems.c 2007-06-22 09:16:00.000000000 +0900
> +++ TMP/trunk/gcc/df-problems.c 2007-06-26 13:53:47.000000000 +0900
> @@ -3713,7 +3713,8 @@ df_set_note (enum reg_note note_type, rt
> }
>
> /* Did not find the note. */
> - REG_NOTES (insn) = alloc_EXPR_LIST (note_type, reg, REG_NOTES (insn));
> + REG_NOTES (insn) = alloc_EXPR_LIST (note_type, copy_rtx (reg),
> + REG_NOTES (insn));
> return old;
> }
>
> Then the above error on SH went away. This one liner may be
> an overkill, but I hope that this experiment will help experts
> to narrow down the problem.
Actually this seems like a correct fix for me. In fact I was just
looking into instance of identical problem on PPC bootstrap (different
file, same directory of libjava :). We will copy iff the reg actually
is subreg and in that case we are required to copy.
I am re-testing PPC targets with this fix in now.
Thanks a lot!
Honza
>
> Regards,
> kaz
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-28 4:24 ` Jan Hubicka
@ 2007-06-28 8:11 ` Paolo Bonzini
2007-06-28 17:16 ` Kenneth Zadeck
1 sibling, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2007-06-28 8:11 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Kaz Kojima, gcc-patches
>> Then the above error on SH went away. This one liner may be
>> an overkill, but I hope that this experiment will help experts
>> to narrow down the problem.
>
> Actually this seems like a correct fix for me. In fact I was just
> looking into instance of identical problem on PPC bootstrap (different
> file, same directory of libjava :). We will copy iff the reg actually
> is subreg and in that case we are required to copy.
Ok, but please let Jan run it thrugh your memory tester first. (Jan, it
may actually be best if you commit it in place of Kaz).
Paolo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-28 4:24 ` Jan Hubicka
2007-06-28 8:11 ` Paolo Bonzini
@ 2007-06-28 17:16 ` Kenneth Zadeck
2007-06-28 18:26 ` Jan Hubicka
2007-06-28 20:19 ` RTL sharing tester (for testing) Paolo Bonzini
1 sibling, 2 replies; 28+ messages in thread
From: Kenneth Zadeck @ 2007-06-28 17:16 UTC (permalink / raw)
To: gcc-patches; +Cc: jh, Bonzini, Paolo, Kaz Kojima, zadeck
>>> Then the above error on SH went away. This one liner may be
>>> an overkill, but I hope that this experiment will help experts
>>> to narrow down the problem.
>>
>> Actually this seems like a correct fix for me. In fact I was just
>> looking into instance of identical problem on PPC bootstrap (different
>> file, same directory of libjava :). We will copy iff the reg actually
>> is subreg and in that case we are required to copy.
>
>Ok, but please let Jan run it thrugh your memory tester first. (Jan, it
>may actually be best if you commit it in place of Kaz).
>
>Paolo
Three things:
(1) I think I would like to get some input from a middle-end
maintainer or a gwm before this patch goes in. This seems like a very
heavy handed solution to this problem. This is going to chew up the
memory.
(2) Kaz, one trick that you can try is to back out this
patch and instead put a call to verify_rtl_sharing right after the
assignment of REG_NOTES in df_set_note.
/* Did not find the note. */
REG_NOTES (insn) = alloc_EXPR_LIST (note_type, reg, REG_NOTES (insn));
verify_rtl_sharing ();
return old;
This will cause a failure when the shared note is inserted in the
stream. If you get into the debugger, the stacktrace should land you
exactly at the place where the shared note is being inserted.
(3) Paolo, I tried the trick in (2) on pr32372 and it did not help.
The reason is that the sharing in cse1 is caused by cse is taking
something out of a reg_equal note and replacing an insn with it
Kenny
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-28 17:16 ` Kenneth Zadeck
@ 2007-06-28 18:26 ` Jan Hubicka
2007-06-28 19:57 ` Kenneth Zadeck
2007-06-28 20:27 ` Richard Sandiford
2007-06-28 20:19 ` RTL sharing tester (for testing) Paolo Bonzini
1 sibling, 2 replies; 28+ messages in thread
From: Jan Hubicka @ 2007-06-28 18:26 UTC (permalink / raw)
To: Kenneth Zadeck; +Cc: gcc-patches, jh, Bonzini, Paolo, Kaz Kojima, zadeck
> >>> Then the above error on SH went away. This one liner may be
> >>> an overkill, but I hope that this experiment will help experts
> >>> to narrow down the problem.
> >>
> >> Actually this seems like a correct fix for me. In fact I was just
> >> looking into instance of identical problem on PPC bootstrap (different
> >> file, same directory of libjava :). We will copy iff the reg actually
> >> is subreg and in that case we are required to copy.
> >
> >Ok, but please let Jan run it thrugh your memory tester first. (Jan, it
> >may actually be best if you commit it in place of Kaz).
> >
> >Paolo
>
> Three things:
>
> (1) I think I would like to get some input from a middle-end
> maintainer or a gwm before this patch goes in. This seems like a very
> heavy handed solution to this problem. This is going to chew up the
> memory.
The users of df_set_note all just take the registers from dataflow
information and call the function to produce REG_DEAD/REG_UNUSED notes.
This all is going to introduce invalid sharing for subregs in all cases
and if we are going to produce the note with SUBREG, we simply must
produce the copy. (If we are very very cautious, we probably can have
some cache since we are probably recomputing those notes quite often,
but I doubt it is worth the extra complexity and we can see it easilly
from memory tester)
Note that copy_rtx does nothing for REG, since registers are shared, so
it should not be overly expensive and we simply must produce the
duplicate for the case of SUBREG or any other unshared RTL expression.
However I now wonder why do we want to see subregs there at all.
Originally I assumed that we now do partial liveness for multiple word
values, but it is not the case. Perhaps the right thing to do is to
simply strip away the SUBREG and keep only the REG itself?
At least before dataflow, I don't think we produced SUBREGs in those
notes, I might be wrong.
>
> (2) Kaz, one trick that you can try is to back out this
> patch and instead put a call to verify_rtl_sharing right after the
> assignment of REG_NOTES in df_set_note.
>
> /* Did not find the note. */
> REG_NOTES (insn) = alloc_EXPR_LIST (note_type, reg, REG_NOTES (insn));
> verify_rtl_sharing ();
> return old;
>
> This will cause a failure when the shared note is inserted in the
> stream. If you get into the debugger, the stacktrace should land you
> exactly at the place where the shared note is being inserted.
>
> (3) Paolo, I tried the trick in (2) on pr32372 and it did not help.
> The reason is that the sharing in cse1 is caused by cse is taking
> something out of a reg_equal note and replacing an insn with it
My method of handling those problems is to simply put verify_rtl_sharing
now and then into the cse_insn spagetti function. Usually the
backtrace from gdb allows quickly to pinpoint the problematic
transformation.
Honza
>
> Kenny
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-28 18:26 ` Jan Hubicka
@ 2007-06-28 19:57 ` Kenneth Zadeck
2007-06-28 20:27 ` Richard Sandiford
1 sibling, 0 replies; 28+ messages in thread
From: Kenneth Zadeck @ 2007-06-28 19:57 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Kenneth Zadeck, gcc-patches, Bonzini, Paolo, Kaz Kojima
Jan Hubicka wrote:
>>>>> Then the above error on SH went away. This one liner may be
>>>>> an overkill, but I hope that this experiment will help experts
>>>>> to narrow down the problem.
>>>>>
>>>> Actually this seems like a correct fix for me. In fact I was just
>>>> looking into instance of identical problem on PPC bootstrap (different
>>>> file, same directory of libjava :). We will copy iff the reg actually
>>>> is subreg and in that case we are required to copy.
>>>>
>>> Ok, but please let Jan run it thrugh your memory tester first. (Jan, it
>>> may actually be best if you commit it in place of Kaz).
>>>
>>> Paolo
>>>
>> Three things:
>>
>> (1) I think I would like to get some input from a middle-end
>> maintainer or a gwm before this patch goes in. This seems like a very
>> heavy handed solution to this problem. This is going to chew up the
>> memory.
>>
>
> The users of df_set_note all just take the registers from dataflow
> information and call the function to produce REG_DEAD/REG_UNUSED notes.
> This all is going to introduce invalid sharing for subregs in all cases
> and if we are going to produce the note with SUBREG, we simply must
> produce the copy. (If we are very very cautious, we probably can have
> some cache since we are probably recomputing those notes quite often,
> but I doubt it is worth the extra complexity and we can see it easilly
> from memory tester)
>
>
Yes, i think that you are right. i was thinking that this was called
when eq_notes were changed. Because of this mistake on my part,
everything I said in this mail is suspect.
> Note that copy_rtx does nothing for REG, since registers are shared, so
> it should not be overly expensive and we simply must produce the
> duplicate for the case of SUBREG or any other unshared RTL expression.
>
> However I now wonder why do we want to see subregs there at all.
> Originally I assumed that we now do partial liveness for multiple word
> values, but it is not the case. Perhaps the right thing to do is to
> simply strip away the SUBREG and keep only the REG itself?
> At least before dataflow, I don't think we produced SUBREGs in those
> notes, I might be wrong.
>
>>
>> (2) Kaz, one trick that you can try is to back out this
>> patch and instead put a call to verify_rtl_sharing right after the
>> assignment of REG_NOTES in df_set_note.
>>
>> /* Did not find the note. */
>> REG_NOTES (insn) = alloc_EXPR_LIST (note_type, reg, REG_NOTES (insn));
>> verify_rtl_sharing ();
>> return old;
>>
>> This will cause a failure when the shared note is inserted in the
>> stream. If you get into the debugger, the stacktrace should land you
>> exactly at the place where the shared note is being inserted.
>>
>> (3) Paolo, I tried the trick in (2) on pr32372 and it did not help.
>> The reason is that the sharing in cse1 is caused by cse is taking
>> something out of a reg_equal note and replacing an insn with it
>>
>
> My method of handling those problems is to simply put verify_rtl_sharing
> now and then into the cse_insn spagetti function. Usually the
> backtrace from gdb allows quickly to pinpoint the problematic
> transformation.
>
> Honza
>
>> Kenny
>>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-28 17:16 ` Kenneth Zadeck
2007-06-28 18:26 ` Jan Hubicka
@ 2007-06-28 20:19 ` Paolo Bonzini
1 sibling, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2007-06-28 20:19 UTC (permalink / raw)
To: Kenneth.Zadeck; +Cc: gcc-patches, jh, Bonzini, Paolo, Kaz Kojima, zadeck
> (3) Paolo, I tried the trick in (2) on pr32372 and it did not help.
> The reason is that the sharing in cse1 is caused by cse is taking
> something out of a reg_equal note and replacing an insn with it
Jan has a patch for PR32372, I think.
Paolo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-28 18:26 ` Jan Hubicka
2007-06-28 19:57 ` Kenneth Zadeck
@ 2007-06-28 20:27 ` Richard Sandiford
2007-06-29 8:29 ` Richard Sandiford
1 sibling, 1 reply; 28+ messages in thread
From: Richard Sandiford @ 2007-06-28 20:27 UTC (permalink / raw)
To: Jan Hubicka
Cc: Kenneth Zadeck, gcc-patches, Bonzini, Paolo, Kaz Kojima, zadeck
Jan Hubicka <jh@suse.cz> writes:
> The users of df_set_note all just take the registers from dataflow
> information and call the function to produce REG_DEAD/REG_UNUSED notes.
> This all is going to introduce invalid sharing for subregs in all cases
> and if we are going to produce the note with SUBREG, we simply must
> produce the copy. (If we are very very cautious, we probably can have
> some cache since we are probably recomputing those notes quite often,
> but I doubt it is worth the extra complexity and we can see it easilly
> from memory tester)
>
> Note that copy_rtx does nothing for REG, since registers are shared, so
> it should not be overly expensive and we simply must produce the
> duplicate for the case of SUBREG or any other unshared RTL expression.
>
> However I now wonder why do we want to see subregs there at all.
> Originally I assumed that we now do partial liveness for multiple word
> values, but it is not the case. Perhaps the right thing to do is to
> simply strip away the SUBREG and keep only the REG itself?
> At least before dataflow, I don't think we produced SUBREGs in those
> notes, I might be wrong.
FWIW, I agree. Several places in gcc seem to assume that REG_DEAD
and REG_UNUSED notes are plain REGs, and access REGNO unconditionally.
It even looks like df_ref_record was written with that in mind;
there's an "oldreg" variable alongside "reg", presumably so that
"reg" could be turned into "SUBREG_REG (oldreg)" if necessary,
but the two variables stay the same throughout.
This was causing a segmentation fault while building libjava
for mipsel-linux-gnu. We had a REG_DEAD (subreg:DI (reg:DF $f0))
note, and some code was applying REGNO to that subreg.
Also, there seems to be some redundancy in df_mw_hardreg.
It has both a mw_reg field and a loc field, but it looks like
all uses of loc can be replaced with uses of mw_reg.
I'm testing the patch below. I wouldn't normally post a patch
before it has finished testing, but since the question is being
actively debated, I thought I might as well.
Richard
Index: gcc/df-scan.c
===================================================================
--- gcc/df-scan.c (revision 126079)
+++ gcc/df-scan.c (working copy)
@@ -2647,6 +2647,7 @@ df_ref_record (struct df_collection_rec
regno += subreg_regno_offset (regno, GET_MODE (SUBREG_REG (reg)),
SUBREG_BYTE (reg), GET_MODE (reg));
endregno = regno + subreg_nregs (reg);
+ reg = SUBREG_REG (reg);
}
else
endregno = END_HARD_REGNO (reg);
@@ -2666,7 +2667,6 @@ df_ref_record (struct df_collection_rec
hardreg->type = ref_type;
hardreg->flags = ref_flags;
hardreg->mw_reg = reg;
- hardreg->loc = loc;
hardreg->start_regno = regno;
hardreg->end_regno = endregno - 1;
hardreg->mw_order = df->ref_order++;
Index: gcc/df.h
===================================================================
--- gcc/df.h (revision 126079)
+++ gcc/df.h (working copy)
@@ -311,8 +311,7 @@ struct dataflow
REG_UNUSED notes. */
struct df_mw_hardreg
{
- rtx mw_reg; /* The multiword hardreg. */
- rtx *loc; /* The location of the reg. */
+ rtx mw_reg; /* The multiword REG. */
enum df_ref_type type; /* Used to see if the ref is read or write. */
enum df_ref_flags flags; /* Various flags. */
unsigned int start_regno; /* First word of the multi word subreg. */
Index: gcc/df-problems.c
===================================================================
--- gcc/df-problems.c (revision 126079)
+++ gcc/df-problems.c (working copy)
@@ -3748,7 +3748,7 @@ df_set_unused_notes_for_mw (rtx insn, rt
if (all_dead)
{
unsigned int regno = mws->start_regno;
- old = df_set_note (REG_UNUSED, insn, old, *(mws->loc));
+ old = df_set_note (REG_UNUSED, insn, old, mws->mw_reg);
#ifdef REG_DEAD_DEBUGGING
df_print_note ("adding 1: ", insn, REG_NOTES (insn));
@@ -3814,7 +3814,7 @@ df_set_dead_notes_for_mw (rtx insn, rtx
if (!bitmap_bit_p (do_not_gen, mws->start_regno))
{
/* Add a dead note for the entire multi word register. */
- old = df_set_note (REG_DEAD, insn, old, *(mws->loc));
+ old = df_set_note (REG_DEAD, insn, old, mws->mw_reg);
#ifdef REG_DEAD_DEBUGGING
df_print_note ("adding 1: ", insn, REG_NOTES (insn));
#endif
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-28 0:48 ` Jan Hubicka
@ 2007-06-28 22:24 ` Eric Christopher
2007-06-29 23:14 ` Graham Stott
1 sibling, 0 replies; 28+ messages in thread
From: Eric Christopher @ 2007-06-28 22:24 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Mark Mitchell, Diego Novillo, Jan Hubicka, gcc-patches
On Jun 27, 2007, at 4:06 PM, Jan Hubicka wrote:
> Sure, I am attaching version I currently use for testing.
> Thanks a lot! (with a bit of luck i686 at least should go smoothly)
OK, I'm running with this now. In my every day work I haven't seen a
problem and don't really notice any appreciable slowdown either.
I say we go with it.
-eric
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-28 20:27 ` Richard Sandiford
@ 2007-06-29 8:29 ` Richard Sandiford
2007-06-29 12:11 ` Kenneth Zadeck
` (2 more replies)
0 siblings, 3 replies; 28+ messages in thread
From: Richard Sandiford @ 2007-06-29 8:29 UTC (permalink / raw)
To: Jan Hubicka
Cc: Kenneth Zadeck, gcc-patches, Bonzini, Paolo, Kaz Kojima, zadeck
Richard Sandiford <rsandifo@nildram.co.uk> writes:
> Jan Hubicka <jh@suse.cz> writes:
>> The users of df_set_note all just take the registers from dataflow
>> information and call the function to produce REG_DEAD/REG_UNUSED notes.
>> This all is going to introduce invalid sharing for subregs in all cases
>> and if we are going to produce the note with SUBREG, we simply must
>> produce the copy. (If we are very very cautious, we probably can have
>> some cache since we are probably recomputing those notes quite often,
>> but I doubt it is worth the extra complexity and we can see it easilly
>> from memory tester)
>>
>> Note that copy_rtx does nothing for REG, since registers are shared, so
>> it should not be overly expensive and we simply must produce the
>> duplicate for the case of SUBREG or any other unshared RTL expression.
>>
>> However I now wonder why do we want to see subregs there at all.
>> Originally I assumed that we now do partial liveness for multiple word
>> values, but it is not the case. Perhaps the right thing to do is to
>> simply strip away the SUBREG and keep only the REG itself?
>> At least before dataflow, I don't think we produced SUBREGs in those
>> notes, I might be wrong.
>
> FWIW, I agree. Several places in gcc seem to assume that REG_DEAD
> and REG_UNUSED notes are plain REGs, and access REGNO unconditionally.
> It even looks like df_ref_record was written with that in mind;
> there's an "oldreg" variable alongside "reg", presumably so that
> "reg" could be turned into "SUBREG_REG (oldreg)" if necessary,
> but the two variables stay the same throughout.
>
> This was causing a segmentation fault while building libjava
> for mipsel-linux-gnu. We had a REG_DEAD (subreg:DI (reg:DF $f0))
> note, and some code was applying REGNO to that subreg.
>
> Also, there seems to be some redundancy in df_mw_hardreg.
> It has both a mw_reg field and a loc field, but it looks like
> all uses of loc can be replaced with uses of mw_reg.
>
> I'm testing the patch below. I wouldn't normally post a patch
> before it has finished testing, but since the question is being
> actively debated, I thought I might as well.
Of course, the problem with doing that is that I changed my mind
soon afterwards. The patch survived testing, but as discussed
on #gcc yesterday, I no longer think using the inner REG is
unconditionally the right thing to do.
The SUBREG handling in df_ref_record assumes that we can ask the port
for the range of registers that (subreg:M (reg:N R)) occupies, even when
mode M is not valid for hard register R. This assumption was inherited
from flow, and I'm assuming that it is correct. (The bug I'm fixing is
a malformed register note rather than inaccurate liveness information.)
The range of registers for M is obviously not always going to be the
same as the range for N, even though it was in the MIPS case.
The code that adds notes will try to use the original mw_reg if
all of the referenced register is dead (for REG_DEAD) or unused
(for REG_UNUSED). It will otherwise create notes for each individual
hard register.
I think we should always take the latter approach -- creating individual
notes for each hard register -- when the mw_reg is a SUBREG rather than
a REG. This ties in quite naturally with the DF_REF_PARTIAL setting;
as far as I can tell, we always consider a reference to a SUBREG of a
multiword hard register to be partial, even if the SUBREG appears to be
the same width as the inner register. (And if we decide to drop the
DF_REF_PARTIAL in some cases, the approach taken in the patch below
should still work for the cases that remain DF_REF_PARTIAL.)
Kenny mentioned the old flow code on #gcc yesterday. I think flow
used to treat a _use_ of a SUBREG like a use of the whole SUBREG_REG.
From mark_used_regs:
case SUBREG:
#ifdef CANNOT_CHANGE_MODE_CLASS
if (flags & PROP_REG_INFO)
record_subregs_of_mode (x);
#endif
/* While we're here, optimize this case. */
x = SUBREG_REG (x);
if (!REG_P (x))
goto retry;
/* Fall through. */
case REG:
/* See a register other than being set => mark it as needed. */
mark_used_reg (pbi, x, cond, insn);
return;
df is now trying to be more accurate by tracking individual hard
registers in the SUBREG_REG. The notes we add must clearly agree
with the main df liveness information, so I don't think the old
flow code sets any precedent here. We're now tracking more
registers individually, so adding more individual notes seems
fair game.
Flow used to track sets individually. From mark_sets_1:
case SUBREG:
if (REG_P (SUBREG_REG (reg)))
{
enum machine_mode outer_mode = GET_MODE (reg);
enum machine_mode inner_mode = GET_MODE (SUBREG_REG (reg));
/* Identify the range of registers affected. This is moderately
tricky for hard registers. See alter_subreg. */
regno_last = regno_first = REGNO (SUBREG_REG (reg));
if (regno_first < FIRST_PSEUDO_REGISTER)
{
regno_first += subreg_regno_offset (regno_first, inner_mode,
SUBREG_BYTE (reg),
outer_mode);
regno_last = (regno_first
+ hard_regno_nregs[regno_first][outer_mode] - 1);
/* Since we've just adjusted the register number ranges, make
sure REG matches. Otherwise some_was_live will be clear
when it shouldn't have been, and we'll create incorrect
REG_UNUSED notes. */
reg = gen_rtx_REG (outer_mode, regno_first);
}
If every register in [regno_first, regno_last] was unused, flow would
add a REG_UNUSED note for the REG created above. If only some of those
registers were unused, flow would add notes for the individual unused
registers. As I said on IRC yesterday, I'm not convinced the first
behaviour was a good idea. That REG might well be invalid (otherwise
why have a SUBREG in the first place?) and it's better not to have
invalid registers floating around.
Bootstrapped & regression-tested on x86_64-linux-gnu. Also tested by
building mipsel-linux-gnu (which failed before the patch). OK to install?
Richard
gcc/
* df.h (df_mw_hardreg): Remove "loc" field.
* df-scan.c (df_ref_record): Don't set it. Remove redundant
local variable.
* df-problems.c (df_whole_mw_reg_unused_p): New function,
split out from df_set_unused_notes_for_mw. Return false for
partial references. Assert that mw_reg is a REG when returning true.
(df_set_unused_notes_for_mw): Use it. Use mw_reg instead of *loc.
(df_whole_mw_reg_dead_p): New function, split out from
df_set_dead_notes_for_mw. Return false for partial references.
Assert that mw_reg is a REG when returning true.
(df_set_dead_notes_for_mw): Use it. Use mw_reg instead of *loc.
Remove redundant bitmap check.
Index: gcc/df.h
===================================================================
--- gcc/df.h (revision 126108)
+++ gcc/df.h (working copy)
@@ -312,7 +312,6 @@ struct dataflow
struct df_mw_hardreg
{
rtx mw_reg; /* The multiword hardreg. */
- rtx *loc; /* The location of the reg. */
enum df_ref_type type; /* Used to see if the ref is read or write. */
enum df_ref_flags flags; /* Various flags. */
unsigned int start_regno; /* First word of the multi word subreg. */
Index: gcc/df-scan.c
===================================================================
--- gcc/df-scan.c (revision 126108)
+++ gcc/df-scan.c (working copy)
@@ -2627,7 +2627,6 @@ df_ref_record (struct df_collection_rec
enum df_ref_type ref_type,
enum df_ref_flags ref_flags)
{
- rtx oldreg = reg;
unsigned int regno;
gcc_assert (REG_P (reg) || GET_CODE (reg) == SUBREG);
@@ -2658,7 +2657,7 @@ df_ref_record (struct df_collection_rec
{
/* Sets to a subreg of a multiword register are partial.
Sets to a non-subreg of a multiword register are not. */
- if (GET_CODE (oldreg) == SUBREG)
+ if (GET_CODE (reg) == SUBREG)
ref_flags |= DF_REF_PARTIAL;
ref_flags |= DF_REF_MW_HARDREG;
@@ -2666,7 +2665,6 @@ df_ref_record (struct df_collection_rec
hardreg->type = ref_type;
hardreg->flags = ref_flags;
hardreg->mw_reg = reg;
- hardreg->loc = loc;
hardreg->start_regno = regno;
hardreg->end_regno = endregno - 1;
hardreg->mw_order = df->ref_order++;
Index: gcc/df-problems.c
===================================================================
--- gcc/df-problems.c (revision 126109)
+++ gcc/df-problems.c (working copy)
@@ -3717,6 +3717,32 @@ df_set_note (enum reg_note note_type, rt
return old;
}
+/* A subroutine of df_set_unused_notes_for_mw, with a selection of its
+ arguments. Return true if the register value described by MWS's
+ mw_reg is known to be completely unused, and if mw_reg can therefore
+ be used in a REG_UNUSED note. */
+
+static bool
+df_whole_mw_reg_unused_p (struct df_mw_hardreg *mws,
+ bitmap live, bitmap artificial_uses)
+{
+ unsigned int r;
+
+ /* If MWS describes a partial reference, create REG_UNUSED notes for
+ individual hard registers. */
+ if (mws->flags & DF_REF_PARTIAL)
+ return false;
+
+ /* Likewise if some part of the register is used. */
+ for (r = mws->start_regno; r <= mws->end_regno; r++)
+ if (bitmap_bit_p (live, r)
+ || bitmap_bit_p (artificial_uses, r))
+ return false;
+
+ gcc_assert (REG_P (mws->mw_reg));
+ return true;
+}
+
/* Set the REG_UNUSED notes for the multiword hardreg defs in INSN
based on the bits in LIVE. Do not generate notes for registers in
artificial uses. DO_NOT_GEN is updated so that REG_DEAD notes are
@@ -3729,7 +3755,6 @@ df_set_unused_notes_for_mw (rtx insn, rt
bitmap live, bitmap do_not_gen,
bitmap artificial_uses)
{
- bool all_dead = true;
unsigned int r;
#ifdef REG_DEAD_DEBUGGING
@@ -3737,18 +3762,11 @@ df_set_unused_notes_for_mw (rtx insn, rt
fprintf (dump_file, "mw_set_unused looking at mws[%d..%d]\n",
mws->start_regno, mws->end_regno);
#endif
- for (r=mws->start_regno; r <= mws->end_regno; r++)
- if ((bitmap_bit_p (live, r))
- || bitmap_bit_p (artificial_uses, r))
- {
- all_dead = false;
- break;
- }
-
- if (all_dead)
+
+ if (df_whole_mw_reg_unused_p (mws, live, artificial_uses))
{
unsigned int regno = mws->start_regno;
- old = df_set_note (REG_UNUSED, insn, old, *(mws->loc));
+ old = df_set_note (REG_UNUSED, insn, old, mws->mw_reg);
#ifdef REG_DEAD_DEBUGGING
df_print_note ("adding 1: ", insn, REG_NOTES (insn));
@@ -3773,6 +3791,34 @@ df_set_unused_notes_for_mw (rtx insn, rt
}
+/* A subroutine of df_set_dead_notes_for_mw, with a selection of its
+ arguments. Return true if the register value described by MWS's
+ mw_reg is known to be completely dead, and if mw_reg can therefore
+ be used in a REG_DEAD note. */
+
+static bool
+df_whole_mw_reg_dead_p (struct df_mw_hardreg *mws,
+ bitmap live, bitmap artificial_uses,
+ bitmap do_not_gen)
+{
+ unsigned int r;
+
+ /* If MWS describes a partial reference, create REG_DEAD notes for
+ individual hard registers. */
+ if (mws->flags & DF_REF_PARTIAL)
+ return false;
+
+ /* Likewise if some part of the register is not dead. */
+ for (r = mws->start_regno; r <= mws->end_regno; r++)
+ if (bitmap_bit_p (live, r)
+ || bitmap_bit_p (artificial_uses, r)
+ || bitmap_bit_p (do_not_gen, r))
+ return false;
+
+ gcc_assert (REG_P (mws->mw_reg));
+ return true;
+}
+
/* Set the REG_DEAD notes for the multiword hardreg use in INSN based
on the bits in LIVE. DO_NOT_GEN is used to keep REG_DEAD notes
from being set if the instruction both reads and writes the
@@ -3783,7 +3829,6 @@ df_set_dead_notes_for_mw (rtx insn, rtx
bitmap live, bitmap do_not_gen,
bitmap artificial_uses)
{
- bool all_dead = true;
unsigned int r;
#ifdef REG_DEAD_DEBUGGING
@@ -3799,25 +3844,13 @@ df_set_dead_notes_for_mw (rtx insn, rtx
}
#endif
- for (r = mws->start_regno; r <= mws->end_regno; r++)
- if ((bitmap_bit_p (live, r))
- || bitmap_bit_p (artificial_uses, r)
- || bitmap_bit_p (do_not_gen, r))
- {
- all_dead = false;
- break;
- }
-
- if (all_dead)
+ if (df_whole_mw_reg_dead_p (mws, live, artificial_uses, do_not_gen))
{
- if (!bitmap_bit_p (do_not_gen, mws->start_regno))
- {
- /* Add a dead note for the entire multi word register. */
- old = df_set_note (REG_DEAD, insn, old, *(mws->loc));
+ /* Add a dead note for the entire multi word register. */
+ old = df_set_note (REG_DEAD, insn, old, mws->mw_reg);
#ifdef REG_DEAD_DEBUGGING
- df_print_note ("adding 1: ", insn, REG_NOTES (insn));
+ df_print_note ("adding 1: ", insn, REG_NOTES (insn));
#endif
- }
}
else
{
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-29 8:29 ` Richard Sandiford
@ 2007-06-29 12:11 ` Kenneth Zadeck
2007-06-29 13:35 ` Paolo Bonzini
2007-06-29 13:37 ` Bernd Schmidt
2007-07-13 10:15 ` RFA: Stop df from generating SUBREG REG_NOTES Richard Sandiford
2 siblings, 1 reply; 28+ messages in thread
From: Kenneth Zadeck @ 2007-06-29 12:11 UTC (permalink / raw)
To: Jan Hubicka, Kenneth Zadeck, gcc-patches, Bonzini, Paolo,
Kaz Kojima, zadeck, rsandifo
Richard Sandiford wrote:
Assuming that this is the same patch as the second patch you sent last
night, it passes on both ppc64 and ia-64.
As i said before, I really do not understand the subtleties here and i
do not believe that Danny or Seongbae do either. This patch will live
or die based on your convincing either Bonzini or a middle end maintainer.
Kenny
> Richard Sandiford <rsandifo@nildram.co.uk> writes:
>
>> Jan Hubicka <jh@suse.cz> writes:
>>
>>> The users of df_set_note all just take the registers from dataflow
>>> information and call the function to produce REG_DEAD/REG_UNUSED notes.
>>> This all is going to introduce invalid sharing for subregs in all cases
>>> and if we are going to produce the note with SUBREG, we simply must
>>> produce the copy. (If we are very very cautious, we probably can have
>>> some cache since we are probably recomputing those notes quite often,
>>> but I doubt it is worth the extra complexity and we can see it easilly
>>> from memory tester)
>>>
>>> Note that copy_rtx does nothing for REG, since registers are shared, so
>>> it should not be overly expensive and we simply must produce the
>>> duplicate for the case of SUBREG or any other unshared RTL expression.
>>>
>>> However I now wonder why do we want to see subregs there at all.
>>> Originally I assumed that we now do partial liveness for multiple word
>>> values, but it is not the case. Perhaps the right thing to do is to
>>> simply strip away the SUBREG and keep only the REG itself?
>>> At least before dataflow, I don't think we produced SUBREGs in those
>>> notes, I might be wrong.
>>>
>> FWIW, I agree. Several places in gcc seem to assume that REG_DEAD
>> and REG_UNUSED notes are plain REGs, and access REGNO unconditionally.
>> It even looks like df_ref_record was written with that in mind;
>> there's an "oldreg" variable alongside "reg", presumably so that
>> "reg" could be turned into "SUBREG_REG (oldreg)" if necessary,
>> but the two variables stay the same throughout.
>>
>> This was causing a segmentation fault while building libjava
>> for mipsel-linux-gnu. We had a REG_DEAD (subreg:DI (reg:DF $f0))
>> note, and some code was applying REGNO to that subreg.
>>
>> Also, there seems to be some redundancy in df_mw_hardreg.
>> It has both a mw_reg field and a loc field, but it looks like
>> all uses of loc can be replaced with uses of mw_reg.
>>
>> I'm testing the patch below. I wouldn't normally post a patch
>> before it has finished testing, but since the question is being
>> actively debated, I thought I might as well.
>>
>
> Of course, the problem with doing that is that I changed my mind
> soon afterwards. The patch survived testing, but as discussed
> on #gcc yesterday, I no longer think using the inner REG is
> unconditionally the right thing to do.
>
> The SUBREG handling in df_ref_record assumes that we can ask the port
> for the range of registers that (subreg:M (reg:N R)) occupies, even when
> mode M is not valid for hard register R. This assumption was inherited
> from flow, and I'm assuming that it is correct. (The bug I'm fixing is
> a malformed register note rather than inaccurate liveness information.)
> The range of registers for M is obviously not always going to be the
> same as the range for N, even though it was in the MIPS case.
>
> The code that adds notes will try to use the original mw_reg if
> all of the referenced register is dead (for REG_DEAD) or unused
> (for REG_UNUSED). It will otherwise create notes for each individual
> hard register.
>
> I think we should always take the latter approach -- creating individual
> notes for each hard register -- when the mw_reg is a SUBREG rather than
> a REG. This ties in quite naturally with the DF_REF_PARTIAL setting;
> as far as I can tell, we always consider a reference to a SUBREG of a
> multiword hard register to be partial, even if the SUBREG appears to be
> the same width as the inner register. (And if we decide to drop the
> DF_REF_PARTIAL in some cases, the approach taken in the patch below
> should still work for the cases that remain DF_REF_PARTIAL.)
>
> Kenny mentioned the old flow code on #gcc yesterday. I think flow
> used to treat a _use_ of a SUBREG like a use of the whole SUBREG_REG.
> From mark_used_regs:
>
> case SUBREG:
> #ifdef CANNOT_CHANGE_MODE_CLASS
> if (flags & PROP_REG_INFO)
> record_subregs_of_mode (x);
> #endif
>
> /* While we're here, optimize this case. */
> x = SUBREG_REG (x);
> if (!REG_P (x))
> goto retry;
> /* Fall through. */
>
> case REG:
> /* See a register other than being set => mark it as needed. */
> mark_used_reg (pbi, x, cond, insn);
> return;
>
> df is now trying to be more accurate by tracking individual hard
> registers in the SUBREG_REG. The notes we add must clearly agree
> with the main df liveness information, so I don't think the old
> flow code sets any precedent here. We're now tracking more
> registers individually, so adding more individual notes seems
> fair game.
>
> Flow used to track sets individually. From mark_sets_1:
>
> case SUBREG:
> if (REG_P (SUBREG_REG (reg)))
> {
> enum machine_mode outer_mode = GET_MODE (reg);
> enum machine_mode inner_mode = GET_MODE (SUBREG_REG (reg));
>
> /* Identify the range of registers affected. This is moderately
> tricky for hard registers. See alter_subreg. */
>
> regno_last = regno_first = REGNO (SUBREG_REG (reg));
> if (regno_first < FIRST_PSEUDO_REGISTER)
> {
> regno_first += subreg_regno_offset (regno_first, inner_mode,
> SUBREG_BYTE (reg),
> outer_mode);
> regno_last = (regno_first
> + hard_regno_nregs[regno_first][outer_mode] - 1);
>
> /* Since we've just adjusted the register number ranges, make
> sure REG matches. Otherwise some_was_live will be clear
> when it shouldn't have been, and we'll create incorrect
> REG_UNUSED notes. */
> reg = gen_rtx_REG (outer_mode, regno_first);
> }
>
> If every register in [regno_first, regno_last] was unused, flow would
> add a REG_UNUSED note for the REG created above. If only some of those
> registers were unused, flow would add notes for the individual unused
> registers. As I said on IRC yesterday, I'm not convinced the first
> behaviour was a good idea. That REG might well be invalid (otherwise
> why have a SUBREG in the first place?) and it's better not to have
> invalid registers floating around.
>
> Bootstrapped & regression-tested on x86_64-linux-gnu. Also tested by
> building mipsel-linux-gnu (which failed before the patch). OK to install?
>
> Richard
>
>
> gcc/
> * df.h (df_mw_hardreg): Remove "loc" field.
> * df-scan.c (df_ref_record): Don't set it. Remove redundant
> local variable.
> * df-problems.c (df_whole_mw_reg_unused_p): New function,
> split out from df_set_unused_notes_for_mw. Return false for
> partial references. Assert that mw_reg is a REG when returning true.
> (df_set_unused_notes_for_mw): Use it. Use mw_reg instead of *loc.
> (df_whole_mw_reg_dead_p): New function, split out from
> df_set_dead_notes_for_mw. Return false for partial references.
> Assert that mw_reg is a REG when returning true.
> (df_set_dead_notes_for_mw): Use it. Use mw_reg instead of *loc.
> Remove redundant bitmap check.
>
> Index: gcc/df.h
> ===================================================================
> --- gcc/df.h (revision 126108)
> +++ gcc/df.h (working copy)
> @@ -312,7 +312,6 @@ struct dataflow
> struct df_mw_hardreg
> {
> rtx mw_reg; /* The multiword hardreg. */
> - rtx *loc; /* The location of the reg. */
> enum df_ref_type type; /* Used to see if the ref is read or write. */
> enum df_ref_flags flags; /* Various flags. */
> unsigned int start_regno; /* First word of the multi word subreg. */
> Index: gcc/df-scan.c
> ===================================================================
> --- gcc/df-scan.c (revision 126108)
> +++ gcc/df-scan.c (working copy)
> @@ -2627,7 +2627,6 @@ df_ref_record (struct df_collection_rec
> enum df_ref_type ref_type,
> enum df_ref_flags ref_flags)
> {
> - rtx oldreg = reg;
> unsigned int regno;
>
> gcc_assert (REG_P (reg) || GET_CODE (reg) == SUBREG);
> @@ -2658,7 +2657,7 @@ df_ref_record (struct df_collection_rec
> {
> /* Sets to a subreg of a multiword register are partial.
> Sets to a non-subreg of a multiword register are not. */
> - if (GET_CODE (oldreg) == SUBREG)
> + if (GET_CODE (reg) == SUBREG)
> ref_flags |= DF_REF_PARTIAL;
> ref_flags |= DF_REF_MW_HARDREG;
>
> @@ -2666,7 +2665,6 @@ df_ref_record (struct df_collection_rec
> hardreg->type = ref_type;
> hardreg->flags = ref_flags;
> hardreg->mw_reg = reg;
> - hardreg->loc = loc;
> hardreg->start_regno = regno;
> hardreg->end_regno = endregno - 1;
> hardreg->mw_order = df->ref_order++;
> Index: gcc/df-problems.c
> ===================================================================
> --- gcc/df-problems.c (revision 126109)
> +++ gcc/df-problems.c (working copy)
> @@ -3717,6 +3717,32 @@ df_set_note (enum reg_note note_type, rt
> return old;
> }
>
> +/* A subroutine of df_set_unused_notes_for_mw, with a selection of its
> + arguments. Return true if the register value described by MWS's
> + mw_reg is known to be completely unused, and if mw_reg can therefore
> + be used in a REG_UNUSED note. */
> +
> +static bool
> +df_whole_mw_reg_unused_p (struct df_mw_hardreg *mws,
> + bitmap live, bitmap artificial_uses)
> +{
> + unsigned int r;
> +
> + /* If MWS describes a partial reference, create REG_UNUSED notes for
> + individual hard registers. */
> + if (mws->flags & DF_REF_PARTIAL)
> + return false;
> +
> + /* Likewise if some part of the register is used. */
> + for (r = mws->start_regno; r <= mws->end_regno; r++)
> + if (bitmap_bit_p (live, r)
> + || bitmap_bit_p (artificial_uses, r))
> + return false;
> +
> + gcc_assert (REG_P (mws->mw_reg));
> + return true;
> +}
> +
> /* Set the REG_UNUSED notes for the multiword hardreg defs in INSN
> based on the bits in LIVE. Do not generate notes for registers in
> artificial uses. DO_NOT_GEN is updated so that REG_DEAD notes are
> @@ -3729,7 +3755,6 @@ df_set_unused_notes_for_mw (rtx insn, rt
> bitmap live, bitmap do_not_gen,
> bitmap artificial_uses)
> {
> - bool all_dead = true;
> unsigned int r;
>
> #ifdef REG_DEAD_DEBUGGING
> @@ -3737,18 +3762,11 @@ df_set_unused_notes_for_mw (rtx insn, rt
> fprintf (dump_file, "mw_set_unused looking at mws[%d..%d]\n",
> mws->start_regno, mws->end_regno);
> #endif
> - for (r=mws->start_regno; r <= mws->end_regno; r++)
> - if ((bitmap_bit_p (live, r))
> - || bitmap_bit_p (artificial_uses, r))
> - {
> - all_dead = false;
> - break;
> - }
> -
> - if (all_dead)
> +
> + if (df_whole_mw_reg_unused_p (mws, live, artificial_uses))
> {
> unsigned int regno = mws->start_regno;
> - old = df_set_note (REG_UNUSED, insn, old, *(mws->loc));
> + old = df_set_note (REG_UNUSED, insn, old, mws->mw_reg);
>
> #ifdef REG_DEAD_DEBUGGING
> df_print_note ("adding 1: ", insn, REG_NOTES (insn));
> @@ -3773,6 +3791,34 @@ df_set_unused_notes_for_mw (rtx insn, rt
> }
>
>
> +/* A subroutine of df_set_dead_notes_for_mw, with a selection of its
> + arguments. Return true if the register value described by MWS's
> + mw_reg is known to be completely dead, and if mw_reg can therefore
> + be used in a REG_DEAD note. */
> +
> +static bool
> +df_whole_mw_reg_dead_p (struct df_mw_hardreg *mws,
> + bitmap live, bitmap artificial_uses,
> + bitmap do_not_gen)
> +{
> + unsigned int r;
> +
> + /* If MWS describes a partial reference, create REG_DEAD notes for
> + individual hard registers. */
> + if (mws->flags & DF_REF_PARTIAL)
> + return false;
> +
> + /* Likewise if some part of the register is not dead. */
> + for (r = mws->start_regno; r <= mws->end_regno; r++)
> + if (bitmap_bit_p (live, r)
> + || bitmap_bit_p (artificial_uses, r)
> + || bitmap_bit_p (do_not_gen, r))
> + return false;
> +
> + gcc_assert (REG_P (mws->mw_reg));
> + return true;
> +}
> +
> /* Set the REG_DEAD notes for the multiword hardreg use in INSN based
> on the bits in LIVE. DO_NOT_GEN is used to keep REG_DEAD notes
> from being set if the instruction both reads and writes the
> @@ -3783,7 +3829,6 @@ df_set_dead_notes_for_mw (rtx insn, rtx
> bitmap live, bitmap do_not_gen,
> bitmap artificial_uses)
> {
> - bool all_dead = true;
> unsigned int r;
>
> #ifdef REG_DEAD_DEBUGGING
> @@ -3799,25 +3844,13 @@ df_set_dead_notes_for_mw (rtx insn, rtx
> }
> #endif
>
> - for (r = mws->start_regno; r <= mws->end_regno; r++)
> - if ((bitmap_bit_p (live, r))
> - || bitmap_bit_p (artificial_uses, r)
> - || bitmap_bit_p (do_not_gen, r))
> - {
> - all_dead = false;
> - break;
> - }
> -
> - if (all_dead)
> + if (df_whole_mw_reg_dead_p (mws, live, artificial_uses, do_not_gen))
> {
> - if (!bitmap_bit_p (do_not_gen, mws->start_regno))
> - {
> - /* Add a dead note for the entire multi word register. */
> - old = df_set_note (REG_DEAD, insn, old, *(mws->loc));
> + /* Add a dead note for the entire multi word register. */
> + old = df_set_note (REG_DEAD, insn, old, mws->mw_reg);
> #ifdef REG_DEAD_DEBUGGING
> - df_print_note ("adding 1: ", insn, REG_NOTES (insn));
> + df_print_note ("adding 1: ", insn, REG_NOTES (insn));
> #endif
> - }
> }
> else
> {
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-29 12:11 ` Kenneth Zadeck
@ 2007-06-29 13:35 ` Paolo Bonzini
0 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2007-06-29 13:35 UTC (permalink / raw)
To: Kenneth Zadeck
Cc: Jan Hubicka, Kenneth Zadeck, gcc-patches, Kaz Kojima, rsandifo
Kenneth Zadeck wrote:
> Richard Sandiford wrote:
>
> Assuming that this is the same patch as the second patch you sent last
> night, it passes on both ppc64 and ia-64.
>
> As i said before, I really do not understand the subtleties here and i
> do not believe that Danny or Seongbae do either. This patch will live
> or die based on your convincing either Bonzini or a middle end maintainer.
I will defer too, since I never did any substantial hacking of flow.c;
in fact I think that in turn Richard understands these subtleties much
better than I do.
Paolo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-29 8:29 ` Richard Sandiford
2007-06-29 12:11 ` Kenneth Zadeck
@ 2007-06-29 13:37 ` Bernd Schmidt
2007-06-29 13:39 ` Jan Hubicka
2007-07-13 10:15 ` RFA: Stop df from generating SUBREG REG_NOTES Richard Sandiford
2 siblings, 1 reply; 28+ messages in thread
From: Bernd Schmidt @ 2007-06-29 13:37 UTC (permalink / raw)
To: Jan Hubicka, Kenneth Zadeck, gcc-patches, Bonzini, Paolo,
Kaz Kojima, zadeck, rsandifo
Richard Sandiford wrote:
> If every register in [regno_first, regno_last] was unused, flow would
> add a REG_UNUSED note for the REG created above. If only some of those
> registers were unused, flow would add notes for the individual unused
> registers. As I said on IRC yesterday, I'm not convinced the first
> behaviour was a good idea. That REG might well be invalid (otherwise
> why have a SUBREG in the first place?) and it's better not to have
> invalid registers floating around.
I agree with everything in your analysis, except possibly this. If the
SUBREG is valid, then I see no way how the REG could be invalid. Since
flow used to do this, I prefer we keep the same behaviour.
Bernd
--
This footer brought to you by insane German lawmakers.
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-29 13:37 ` Bernd Schmidt
@ 2007-06-29 13:39 ` Jan Hubicka
2007-06-29 16:55 ` Richard Sandiford
0 siblings, 1 reply; 28+ messages in thread
From: Jan Hubicka @ 2007-06-29 13:39 UTC (permalink / raw)
To: Bernd Schmidt
Cc: Jan Hubicka, Kenneth Zadeck, gcc-patches, Bonzini, Paolo,
Kaz Kojima, zadeck, rsandifo
> Richard Sandiford wrote:
> >If every register in [regno_first, regno_last] was unused, flow would
> >add a REG_UNUSED note for the REG created above. If only some of those
> >registers were unused, flow would add notes for the individual unused
> >registers. As I said on IRC yesterday, I'm not convinced the first
> >behaviour was a good idea. That REG might well be invalid (otherwise
> >why have a SUBREG in the first place?) and it's better not to have
> >invalid registers floating around.
>
> I agree with everything in your analysis, except possibly this. If the
> SUBREG is valid, then I see no way how the REG could be invalid. Since
> flow used to do this, I prefer we keep the same behaviour.
I also agree here, in general we should have REGs floating around
subregs valid. The presence of subreg would be here as a mere retyping
or something. It should not be that common as we take care to fold away
subregs of hard regs in several places. (in longer run we might go
further and declare those foldable subregs invalid, not sure if it is
helping much however)
Adding one big coumpound REG_UNUSED/REG_DEAD note for the whole register
IMO should make difference for peephole2 pass that is doing mathching on
them (I am not sure if any of peep2s in existence rely on multiword
hardregs).
The patch seems to be on right track IMHO :)
BTW with the latest round of fixes, PPC is finally in bootstrapland
with sharing enabled. With clean mainline we have couple of failures in
testsuite, but still an improvement ;)
I also gave that patch testing on i686-linux with sharing checker and it
did pass.
Honza
>
>
> Bernd
> --
> This footer brought to you by insane German lawmakers.
> Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
> Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
> Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-29 13:39 ` Jan Hubicka
@ 2007-06-29 16:55 ` Richard Sandiford
0 siblings, 0 replies; 28+ messages in thread
From: Richard Sandiford @ 2007-06-29 16:55 UTC (permalink / raw)
To: Jan Hubicka
Cc: Bernd Schmidt, Kenneth Zadeck, gcc-patches, Bonzini, Paolo,
Kaz Kojima, zadeck
Jan Hubicka <jh@suse.cz> writes:
>> Richard Sandiford wrote:
>> >If every register in [regno_first, regno_last] was unused, flow would
>> >add a REG_UNUSED note for the REG created above. If only some of those
>> >registers were unused, flow would add notes for the individual unused
>> >registers. As I said on IRC yesterday, I'm not convinced the first
>> >behaviour was a good idea. That REG might well be invalid (otherwise
>> >why have a SUBREG in the first place?) and it's better not to have
>> >invalid registers floating around.
>>
>> I agree with everything in your analysis, except possibly this. If the
>> SUBREG is valid, then I see no way how the REG could be invalid. Since
>> flow used to do this, I prefer we keep the same behaviour.
AIUI, it isn't a requirement though. If we insisted that the equivalent
REG be valid (in the HARD_REGNO_MODE_OK sense), then I don't see any
point in subregs of hard registers at all. The validate_subreg case
for hard registers ought simply to be "return false". But at the moment
it allows subregs in which the equivalent hard register is not valid in
the HARD_REGNO_MODE_OK sense, as long as we can "safely" determine the
range of registers it covers.
This is what is happening in he mipsel-linux-gnu case. We had
(subreg:DI (reg:DF $f0)), where $f0 is valid for DFmode but not DImode.
(And it shouldn't be valid for DImode in this case; we don't have any
instructions that would make a DImode FPR useful.)
I agree it makes sense to forbid subregs of hard registers altogether,
including the MIPS one above (not least because I'm unconvinced how
"safe" the determination really is). But that's going to be quite
a far-reaching change, and isn't something we can likely do in
a few days. In the meantime, I think the patch really does match
the current rules.
> I also agree here, in general we should have REGs floating around
> subregs valid. The presence of subreg would be here as a mere retyping
> or something. It should not be that common as we take care to fold away
> subregs of hard regs in several places. (in longer run we might go
> further and declare those foldable subregs invalid, not sure if it is
> helping much however)
I agree that there's no point in having SUBREGs when an equivalent REG
exists, and that we already try quite hard to avoid them. Were the
SUBREGs that you found examples of ones we'd missed, or didn't they
reduce to a valid REG at all?
> Adding one big coumpound REG_UNUSED/REG_DEAD note for the whole register
> IMO should make difference for peephole2 pass that is doing mathching on
> them (I am not sure if any of peep2s in existence rely on multiword
> hardregs).
Well, I'm not sure we should have such subregs after reload, no.
Hopefully the code you mentioned should ensure that. (The case
I'm dealing with is before reload FWIW.)
Richard
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-28 0:48 ` Jan Hubicka
2007-06-28 22:24 ` Eric Christopher
@ 2007-06-29 23:14 ` Graham Stott
2007-06-30 0:17 ` Jan Hubicka
1 sibling, 1 reply; 28+ messages in thread
From: Graham Stott @ 2007-06-29 23:14 UTC (permalink / raw)
To: Jan Hubicka
Cc: Eric Christopher, Mark Mitchell, Diego Novillo, Jan Hubicka, gcc-patches
Jan,
I've just done a svn update and my enable checking build now
fails because TODO_verify_rtl_sharing is undefined.
It looks like the tree-pass.h change didn't wasn't committed?
Cheers
Graham
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RTL sharing tester (for testing)
2007-06-29 23:14 ` Graham Stott
@ 2007-06-30 0:17 ` Jan Hubicka
0 siblings, 0 replies; 28+ messages in thread
From: Jan Hubicka @ 2007-06-30 0:17 UTC (permalink / raw)
To: Graham Stott
Cc: Jan Hubicka, Eric Christopher, Mark Mitchell, Diego Novillo,
Jan Hubicka, gcc-patches
> Jan,
>
> I've just done a svn update and my enable checking build now
> fails because TODO_verify_rtl_sharing is undefined.
>
> It looks like the tree-pass.h change didn't wasn't committed?
Oops, looks like I must've accidentally comitted from checking enabled
tree. I am going to revert that TODO_verify_rtl_sharing change.
Honza
>
> Cheers
> Graham
^ permalink raw reply [flat|nested] 28+ messages in thread
* RFA: Stop df from generating SUBREG REG_NOTES
2007-06-29 8:29 ` Richard Sandiford
2007-06-29 12:11 ` Kenneth Zadeck
2007-06-29 13:37 ` Bernd Schmidt
@ 2007-07-13 10:15 ` Richard Sandiford
2007-07-25 22:50 ` Richard Sandiford
2007-07-27 0:01 ` Ian Lance Taylor
2 siblings, 2 replies; 28+ messages in thread
From: Richard Sandiford @ 2007-07-13 10:15 UTC (permalink / raw)
To: gcc-patches
Ping!
Richard Sandiford <rsandifo@nildram.co.uk> writes:
> Richard Sandiford <rsandifo@nildram.co.uk> writes:
>> Jan Hubicka <jh@suse.cz> writes:
>>> The users of df_set_note all just take the registers from dataflow
>>> information and call the function to produce REG_DEAD/REG_UNUSED notes.
>>> This all is going to introduce invalid sharing for subregs in all cases
>>> and if we are going to produce the note with SUBREG, we simply must
>>> produce the copy. (If we are very very cautious, we probably can have
>>> some cache since we are probably recomputing those notes quite often,
>>> but I doubt it is worth the extra complexity and we can see it easilly
>>> from memory tester)
>>>
>>> Note that copy_rtx does nothing for REG, since registers are shared, so
>>> it should not be overly expensive and we simply must produce the
>>> duplicate for the case of SUBREG or any other unshared RTL expression.
>>>
>>> However I now wonder why do we want to see subregs there at all.
>>> Originally I assumed that we now do partial liveness for multiple word
>>> values, but it is not the case. Perhaps the right thing to do is to
>>> simply strip away the SUBREG and keep only the REG itself?
>>> At least before dataflow, I don't think we produced SUBREGs in those
>>> notes, I might be wrong.
>>
>> FWIW, I agree. Several places in gcc seem to assume that REG_DEAD
>> and REG_UNUSED notes are plain REGs, and access REGNO unconditionally.
>> It even looks like df_ref_record was written with that in mind;
>> there's an "oldreg" variable alongside "reg", presumably so that
>> "reg" could be turned into "SUBREG_REG (oldreg)" if necessary,
>> but the two variables stay the same throughout.
>>
>> This was causing a segmentation fault while building libjava
>> for mipsel-linux-gnu. We had a REG_DEAD (subreg:DI (reg:DF $f0))
>> note, and some code was applying REGNO to that subreg.
>>
>> Also, there seems to be some redundancy in df_mw_hardreg.
>> It has both a mw_reg field and a loc field, but it looks like
>> all uses of loc can be replaced with uses of mw_reg.
>>
>> I'm testing the patch below. I wouldn't normally post a patch
>> before it has finished testing, but since the question is being
>> actively debated, I thought I might as well.
>
> Of course, the problem with doing that is that I changed my mind
> soon afterwards. The patch survived testing, but as discussed
> on #gcc yesterday, I no longer think using the inner REG is
> unconditionally the right thing to do.
>
> The SUBREG handling in df_ref_record assumes that we can ask the port
> for the range of registers that (subreg:M (reg:N R)) occupies, even when
> mode M is not valid for hard register R. This assumption was inherited
> from flow, and I'm assuming that it is correct. (The bug I'm fixing is
> a malformed register note rather than inaccurate liveness information.)
> The range of registers for M is obviously not always going to be the
> same as the range for N, even though it was in the MIPS case.
>
> The code that adds notes will try to use the original mw_reg if
> all of the referenced register is dead (for REG_DEAD) or unused
> (for REG_UNUSED). It will otherwise create notes for each individual
> hard register.
>
> I think we should always take the latter approach -- creating individual
> notes for each hard register -- when the mw_reg is a SUBREG rather than
> a REG. This ties in quite naturally with the DF_REF_PARTIAL setting;
> as far as I can tell, we always consider a reference to a SUBREG of a
> multiword hard register to be partial, even if the SUBREG appears to be
> the same width as the inner register. (And if we decide to drop the
> DF_REF_PARTIAL in some cases, the approach taken in the patch below
> should still work for the cases that remain DF_REF_PARTIAL.)
>
> Kenny mentioned the old flow code on #gcc yesterday. I think flow
> used to treat a _use_ of a SUBREG like a use of the whole SUBREG_REG.
>> From mark_used_regs:
>
> case SUBREG:
> #ifdef CANNOT_CHANGE_MODE_CLASS
> if (flags & PROP_REG_INFO)
> record_subregs_of_mode (x);
> #endif
>
> /* While we're here, optimize this case. */
> x = SUBREG_REG (x);
> if (!REG_P (x))
> goto retry;
> /* Fall through. */
>
> case REG:
> /* See a register other than being set => mark it as needed. */
> mark_used_reg (pbi, x, cond, insn);
> return;
>
> df is now trying to be more accurate by tracking individual hard
> registers in the SUBREG_REG. The notes we add must clearly agree
> with the main df liveness information, so I don't think the old
> flow code sets any precedent here. We're now tracking more
> registers individually, so adding more individual notes seems
> fair game.
>
> Flow used to track sets individually. From mark_sets_1:
>
> case SUBREG:
> if (REG_P (SUBREG_REG (reg)))
> {
> enum machine_mode outer_mode = GET_MODE (reg);
> enum machine_mode inner_mode = GET_MODE (SUBREG_REG (reg));
>
> /* Identify the range of registers affected. This is moderately
> tricky for hard registers. See alter_subreg. */
>
> regno_last = regno_first = REGNO (SUBREG_REG (reg));
> if (regno_first < FIRST_PSEUDO_REGISTER)
> {
> regno_first += subreg_regno_offset (regno_first, inner_mode,
> SUBREG_BYTE (reg),
> outer_mode);
> regno_last = (regno_first
> + hard_regno_nregs[regno_first][outer_mode] - 1);
>
> /* Since we've just adjusted the register number ranges, make
> sure REG matches. Otherwise some_was_live will be clear
> when it shouldn't have been, and we'll create incorrect
> REG_UNUSED notes. */
> reg = gen_rtx_REG (outer_mode, regno_first);
> }
>
> If every register in [regno_first, regno_last] was unused, flow would
> add a REG_UNUSED note for the REG created above. If only some of those
> registers were unused, flow would add notes for the individual unused
> registers. As I said on IRC yesterday, I'm not convinced the first
> behaviour was a good idea. That REG might well be invalid (otherwise
> why have a SUBREG in the first place?) and it's better not to have
> invalid registers floating around.
>
> Bootstrapped & regression-tested on x86_64-linux-gnu. Also tested by
> building mipsel-linux-gnu (which failed before the patch). OK to install?
>
> Richard
>
>
> gcc/
> * df.h (df_mw_hardreg): Remove "loc" field.
> * df-scan.c (df_ref_record): Don't set it. Remove redundant
> local variable.
> * df-problems.c (df_whole_mw_reg_unused_p): New function,
> split out from df_set_unused_notes_for_mw. Return false for
> partial references. Assert that mw_reg is a REG when returning true.
> (df_set_unused_notes_for_mw): Use it. Use mw_reg instead of *loc.
> (df_whole_mw_reg_dead_p): New function, split out from
> df_set_dead_notes_for_mw. Return false for partial references.
> Assert that mw_reg is a REG when returning true.
> (df_set_dead_notes_for_mw): Use it. Use mw_reg instead of *loc.
> Remove redundant bitmap check.
>
> Index: gcc/df.h
> ===================================================================
> --- gcc/df.h (revision 126108)
> +++ gcc/df.h (working copy)
> @@ -312,7 +312,6 @@ struct dataflow
> struct df_mw_hardreg
> {
> rtx mw_reg; /* The multiword hardreg. */
> - rtx *loc; /* The location of the reg. */
> enum df_ref_type type; /* Used to see if the ref is read or write. */
> enum df_ref_flags flags; /* Various flags. */
> unsigned int start_regno; /* First word of the multi word subreg. */
> Index: gcc/df-scan.c
> ===================================================================
> --- gcc/df-scan.c (revision 126108)
> +++ gcc/df-scan.c (working copy)
> @@ -2627,7 +2627,6 @@ df_ref_record (struct df_collection_rec
> enum df_ref_type ref_type,
> enum df_ref_flags ref_flags)
> {
> - rtx oldreg = reg;
> unsigned int regno;
>
> gcc_assert (REG_P (reg) || GET_CODE (reg) == SUBREG);
> @@ -2658,7 +2657,7 @@ df_ref_record (struct df_collection_rec
> {
> /* Sets to a subreg of a multiword register are partial.
> Sets to a non-subreg of a multiword register are not. */
> - if (GET_CODE (oldreg) == SUBREG)
> + if (GET_CODE (reg) == SUBREG)
> ref_flags |= DF_REF_PARTIAL;
> ref_flags |= DF_REF_MW_HARDREG;
>
> @@ -2666,7 +2665,6 @@ df_ref_record (struct df_collection_rec
> hardreg->type = ref_type;
> hardreg->flags = ref_flags;
> hardreg->mw_reg = reg;
> - hardreg->loc = loc;
> hardreg->start_regno = regno;
> hardreg->end_regno = endregno - 1;
> hardreg->mw_order = df->ref_order++;
> Index: gcc/df-problems.c
> ===================================================================
> --- gcc/df-problems.c (revision 126109)
> +++ gcc/df-problems.c (working copy)
> @@ -3717,6 +3717,32 @@ df_set_note (enum reg_note note_type, rt
> return old;
> }
>
> +/* A subroutine of df_set_unused_notes_for_mw, with a selection of its
> + arguments. Return true if the register value described by MWS's
> + mw_reg is known to be completely unused, and if mw_reg can therefore
> + be used in a REG_UNUSED note. */
> +
> +static bool
> +df_whole_mw_reg_unused_p (struct df_mw_hardreg *mws,
> + bitmap live, bitmap artificial_uses)
> +{
> + unsigned int r;
> +
> + /* If MWS describes a partial reference, create REG_UNUSED notes for
> + individual hard registers. */
> + if (mws->flags & DF_REF_PARTIAL)
> + return false;
> +
> + /* Likewise if some part of the register is used. */
> + for (r = mws->start_regno; r <= mws->end_regno; r++)
> + if (bitmap_bit_p (live, r)
> + || bitmap_bit_p (artificial_uses, r))
> + return false;
> +
> + gcc_assert (REG_P (mws->mw_reg));
> + return true;
> +}
> +
> /* Set the REG_UNUSED notes for the multiword hardreg defs in INSN
> based on the bits in LIVE. Do not generate notes for registers in
> artificial uses. DO_NOT_GEN is updated so that REG_DEAD notes are
> @@ -3729,7 +3755,6 @@ df_set_unused_notes_for_mw (rtx insn, rt
> bitmap live, bitmap do_not_gen,
> bitmap artificial_uses)
> {
> - bool all_dead = true;
> unsigned int r;
>
> #ifdef REG_DEAD_DEBUGGING
> @@ -3737,18 +3762,11 @@ df_set_unused_notes_for_mw (rtx insn, rt
> fprintf (dump_file, "mw_set_unused looking at mws[%d..%d]\n",
> mws->start_regno, mws->end_regno);
> #endif
> - for (r=mws->start_regno; r <= mws->end_regno; r++)
> - if ((bitmap_bit_p (live, r))
> - || bitmap_bit_p (artificial_uses, r))
> - {
> - all_dead = false;
> - break;
> - }
> -
> - if (all_dead)
> +
> + if (df_whole_mw_reg_unused_p (mws, live, artificial_uses))
> {
> unsigned int regno = mws->start_regno;
> - old = df_set_note (REG_UNUSED, insn, old, *(mws->loc));
> + old = df_set_note (REG_UNUSED, insn, old, mws->mw_reg);
>
> #ifdef REG_DEAD_DEBUGGING
> df_print_note ("adding 1: ", insn, REG_NOTES (insn));
> @@ -3773,6 +3791,34 @@ df_set_unused_notes_for_mw (rtx insn, rt
> }
>
>
> +/* A subroutine of df_set_dead_notes_for_mw, with a selection of its
> + arguments. Return true if the register value described by MWS's
> + mw_reg is known to be completely dead, and if mw_reg can therefore
> + be used in a REG_DEAD note. */
> +
> +static bool
> +df_whole_mw_reg_dead_p (struct df_mw_hardreg *mws,
> + bitmap live, bitmap artificial_uses,
> + bitmap do_not_gen)
> +{
> + unsigned int r;
> +
> + /* If MWS describes a partial reference, create REG_DEAD notes for
> + individual hard registers. */
> + if (mws->flags & DF_REF_PARTIAL)
> + return false;
> +
> + /* Likewise if some part of the register is not dead. */
> + for (r = mws->start_regno; r <= mws->end_regno; r++)
> + if (bitmap_bit_p (live, r)
> + || bitmap_bit_p (artificial_uses, r)
> + || bitmap_bit_p (do_not_gen, r))
> + return false;
> +
> + gcc_assert (REG_P (mws->mw_reg));
> + return true;
> +}
> +
> /* Set the REG_DEAD notes for the multiword hardreg use in INSN based
> on the bits in LIVE. DO_NOT_GEN is used to keep REG_DEAD notes
> from being set if the instruction both reads and writes the
> @@ -3783,7 +3829,6 @@ df_set_dead_notes_for_mw (rtx insn, rtx
> bitmap live, bitmap do_not_gen,
> bitmap artificial_uses)
> {
> - bool all_dead = true;
> unsigned int r;
>
> #ifdef REG_DEAD_DEBUGGING
> @@ -3799,25 +3844,13 @@ df_set_dead_notes_for_mw (rtx insn, rtx
> }
> #endif
>
> - for (r = mws->start_regno; r <= mws->end_regno; r++)
> - if ((bitmap_bit_p (live, r))
> - || bitmap_bit_p (artificial_uses, r)
> - || bitmap_bit_p (do_not_gen, r))
> - {
> - all_dead = false;
> - break;
> - }
> -
> - if (all_dead)
> + if (df_whole_mw_reg_dead_p (mws, live, artificial_uses, do_not_gen))
> {
> - if (!bitmap_bit_p (do_not_gen, mws->start_regno))
> - {
> - /* Add a dead note for the entire multi word register. */
> - old = df_set_note (REG_DEAD, insn, old, *(mws->loc));
> + /* Add a dead note for the entire multi word register. */
> + old = df_set_note (REG_DEAD, insn, old, mws->mw_reg);
> #ifdef REG_DEAD_DEBUGGING
> - df_print_note ("adding 1: ", insn, REG_NOTES (insn));
> + df_print_note ("adding 1: ", insn, REG_NOTES (insn));
> #endif
> - }
> }
> else
> {
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RFA: Stop df from generating SUBREG REG_NOTES
2007-07-13 10:15 ` RFA: Stop df from generating SUBREG REG_NOTES Richard Sandiford
@ 2007-07-25 22:50 ` Richard Sandiford
2007-07-27 0:01 ` Ian Lance Taylor
1 sibling, 0 replies; 28+ messages in thread
From: Richard Sandiford @ 2007-07-25 22:50 UTC (permalink / raw)
To: gcc-patches
Ping^2
Cross builds of mips-linux-gnu have been broken since the
dataflow merge, so it would be nice to get this sorted.
Richard
Richard Sandiford <rsandifo@nildram.co.uk> writes:
> Ping!
> Richard Sandiford <rsandifo@nildram.co.uk> writes:
>> Richard Sandiford <rsandifo@nildram.co.uk> writes:
>>> Jan Hubicka <jh@suse.cz> writes:
>>>> The users of df_set_note all just take the registers from dataflow
>>>> information and call the function to produce REG_DEAD/REG_UNUSED notes.
>>>> This all is going to introduce invalid sharing for subregs in all cases
>>>> and if we are going to produce the note with SUBREG, we simply must
>>>> produce the copy. (If we are very very cautious, we probably can have
>>>> some cache since we are probably recomputing those notes quite often,
>>>> but I doubt it is worth the extra complexity and we can see it easilly
>>>> from memory tester)
>>>>
>>>> Note that copy_rtx does nothing for REG, since registers are shared, so
>>>> it should not be overly expensive and we simply must produce the
>>>> duplicate for the case of SUBREG or any other unshared RTL expression.
>>>>
>>>> However I now wonder why do we want to see subregs there at all.
>>>> Originally I assumed that we now do partial liveness for multiple word
>>>> values, but it is not the case. Perhaps the right thing to do is to
>>>> simply strip away the SUBREG and keep only the REG itself?
>>>> At least before dataflow, I don't think we produced SUBREGs in those
>>>> notes, I might be wrong.
>>>
>>> FWIW, I agree. Several places in gcc seem to assume that REG_DEAD
>>> and REG_UNUSED notes are plain REGs, and access REGNO unconditionally.
>>> It even looks like df_ref_record was written with that in mind;
>>> there's an "oldreg" variable alongside "reg", presumably so that
>>> "reg" could be turned into "SUBREG_REG (oldreg)" if necessary,
>>> but the two variables stay the same throughout.
>>>
>>> This was causing a segmentation fault while building libjava
>>> for mipsel-linux-gnu. We had a REG_DEAD (subreg:DI (reg:DF $f0))
>>> note, and some code was applying REGNO to that subreg.
>>>
>>> Also, there seems to be some redundancy in df_mw_hardreg.
>>> It has both a mw_reg field and a loc field, but it looks like
>>> all uses of loc can be replaced with uses of mw_reg.
>>>
>>> I'm testing the patch below. I wouldn't normally post a patch
>>> before it has finished testing, but since the question is being
>>> actively debated, I thought I might as well.
>>
>> Of course, the problem with doing that is that I changed my mind
>> soon afterwards. The patch survived testing, but as discussed
>> on #gcc yesterday, I no longer think using the inner REG is
>> unconditionally the right thing to do.
>>
>> The SUBREG handling in df_ref_record assumes that we can ask the port
>> for the range of registers that (subreg:M (reg:N R)) occupies, even when
>> mode M is not valid for hard register R. This assumption was inherited
>> from flow, and I'm assuming that it is correct. (The bug I'm fixing is
>> a malformed register note rather than inaccurate liveness information.)
>> The range of registers for M is obviously not always going to be the
>> same as the range for N, even though it was in the MIPS case.
>>
>> The code that adds notes will try to use the original mw_reg if
>> all of the referenced register is dead (for REG_DEAD) or unused
>> (for REG_UNUSED). It will otherwise create notes for each individual
>> hard register.
>>
>> I think we should always take the latter approach -- creating individual
>> notes for each hard register -- when the mw_reg is a SUBREG rather than
>> a REG. This ties in quite naturally with the DF_REF_PARTIAL setting;
>> as far as I can tell, we always consider a reference to a SUBREG of a
>> multiword hard register to be partial, even if the SUBREG appears to be
>> the same width as the inner register. (And if we decide to drop the
>> DF_REF_PARTIAL in some cases, the approach taken in the patch below
>> should still work for the cases that remain DF_REF_PARTIAL.)
>>
>> Kenny mentioned the old flow code on #gcc yesterday. I think flow
>> used to treat a _use_ of a SUBREG like a use of the whole SUBREG_REG.
>>> From mark_used_regs:
>>
>> case SUBREG:
>> #ifdef CANNOT_CHANGE_MODE_CLASS
>> if (flags & PROP_REG_INFO)
>> record_subregs_of_mode (x);
>> #endif
>>
>> /* While we're here, optimize this case. */
>> x = SUBREG_REG (x);
>> if (!REG_P (x))
>> goto retry;
>> /* Fall through. */
>>
>> case REG:
>> /* See a register other than being set => mark it as needed. */
>> mark_used_reg (pbi, x, cond, insn);
>> return;
>>
>> df is now trying to be more accurate by tracking individual hard
>> registers in the SUBREG_REG. The notes we add must clearly agree
>> with the main df liveness information, so I don't think the old
>> flow code sets any precedent here. We're now tracking more
>> registers individually, so adding more individual notes seems
>> fair game.
>>
>> Flow used to track sets individually. From mark_sets_1:
>>
>> case SUBREG:
>> if (REG_P (SUBREG_REG (reg)))
>> {
>> enum machine_mode outer_mode = GET_MODE (reg);
>> enum machine_mode inner_mode = GET_MODE (SUBREG_REG (reg));
>>
>> /* Identify the range of registers affected. This is moderately
>> tricky for hard registers. See alter_subreg. */
>>
>> regno_last = regno_first = REGNO (SUBREG_REG (reg));
>> if (regno_first < FIRST_PSEUDO_REGISTER)
>> {
>> regno_first += subreg_regno_offset (regno_first, inner_mode,
>> SUBREG_BYTE (reg),
>> outer_mode);
>> regno_last = (regno_first
>> + hard_regno_nregs[regno_first][outer_mode] - 1);
>>
>> /* Since we've just adjusted the register number ranges, make
>> sure REG matches. Otherwise some_was_live will be clear
>> when it shouldn't have been, and we'll create incorrect
>> REG_UNUSED notes. */
>> reg = gen_rtx_REG (outer_mode, regno_first);
>> }
>>
>> If every register in [regno_first, regno_last] was unused, flow would
>> add a REG_UNUSED note for the REG created above. If only some of those
>> registers were unused, flow would add notes for the individual unused
>> registers. As I said on IRC yesterday, I'm not convinced the first
>> behaviour was a good idea. That REG might well be invalid (otherwise
>> why have a SUBREG in the first place?) and it's better not to have
>> invalid registers floating around.
>>
>> Bootstrapped & regression-tested on x86_64-linux-gnu. Also tested by
>> building mipsel-linux-gnu (which failed before the patch). OK to install?
>>
>> Richard
>>
>>
>> gcc/
>> * df.h (df_mw_hardreg): Remove "loc" field.
>> * df-scan.c (df_ref_record): Don't set it. Remove redundant
>> local variable.
>> * df-problems.c (df_whole_mw_reg_unused_p): New function,
>> split out from df_set_unused_notes_for_mw. Return false for
>> partial references. Assert that mw_reg is a REG when returning true.
>> (df_set_unused_notes_for_mw): Use it. Use mw_reg instead of *loc.
>> (df_whole_mw_reg_dead_p): New function, split out from
>> df_set_dead_notes_for_mw. Return false for partial references.
>> Assert that mw_reg is a REG when returning true.
>> (df_set_dead_notes_for_mw): Use it. Use mw_reg instead of *loc.
>> Remove redundant bitmap check.
>>
>> Index: gcc/df.h
>> ===================================================================
>> --- gcc/df.h (revision 126108)
>> +++ gcc/df.h (working copy)
>> @@ -312,7 +312,6 @@ struct dataflow
>> struct df_mw_hardreg
>> {
>> rtx mw_reg; /* The multiword hardreg. */
>> - rtx *loc; /* The location of the reg. */
>> enum df_ref_type type; /* Used to see if the ref is read or write. */
>> enum df_ref_flags flags; /* Various flags. */
>> unsigned int start_regno; /* First word of the multi word subreg. */
>> Index: gcc/df-scan.c
>> ===================================================================
>> --- gcc/df-scan.c (revision 126108)
>> +++ gcc/df-scan.c (working copy)
>> @@ -2627,7 +2627,6 @@ df_ref_record (struct df_collection_rec
>> enum df_ref_type ref_type,
>> enum df_ref_flags ref_flags)
>> {
>> - rtx oldreg = reg;
>> unsigned int regno;
>>
>> gcc_assert (REG_P (reg) || GET_CODE (reg) == SUBREG);
>> @@ -2658,7 +2657,7 @@ df_ref_record (struct df_collection_rec
>> {
>> /* Sets to a subreg of a multiword register are partial.
>> Sets to a non-subreg of a multiword register are not. */
>> - if (GET_CODE (oldreg) == SUBREG)
>> + if (GET_CODE (reg) == SUBREG)
>> ref_flags |= DF_REF_PARTIAL;
>> ref_flags |= DF_REF_MW_HARDREG;
>>
>> @@ -2666,7 +2665,6 @@ df_ref_record (struct df_collection_rec
>> hardreg->type = ref_type;
>> hardreg->flags = ref_flags;
>> hardreg->mw_reg = reg;
>> - hardreg->loc = loc;
>> hardreg->start_regno = regno;
>> hardreg->end_regno = endregno - 1;
>> hardreg->mw_order = df->ref_order++;
>> Index: gcc/df-problems.c
>> ===================================================================
>> --- gcc/df-problems.c (revision 126109)
>> +++ gcc/df-problems.c (working copy)
>> @@ -3717,6 +3717,32 @@ df_set_note (enum reg_note note_type, rt
>> return old;
>> }
>>
>> +/* A subroutine of df_set_unused_notes_for_mw, with a selection of its
>> + arguments. Return true if the register value described by MWS's
>> + mw_reg is known to be completely unused, and if mw_reg can therefore
>> + be used in a REG_UNUSED note. */
>> +
>> +static bool
>> +df_whole_mw_reg_unused_p (struct df_mw_hardreg *mws,
>> + bitmap live, bitmap artificial_uses)
>> +{
>> + unsigned int r;
>> +
>> + /* If MWS describes a partial reference, create REG_UNUSED notes for
>> + individual hard registers. */
>> + if (mws->flags & DF_REF_PARTIAL)
>> + return false;
>> +
>> + /* Likewise if some part of the register is used. */
>> + for (r = mws->start_regno; r <= mws->end_regno; r++)
>> + if (bitmap_bit_p (live, r)
>> + || bitmap_bit_p (artificial_uses, r))
>> + return false;
>> +
>> + gcc_assert (REG_P (mws->mw_reg));
>> + return true;
>> +}
>> +
>> /* Set the REG_UNUSED notes for the multiword hardreg defs in INSN
>> based on the bits in LIVE. Do not generate notes for registers in
>> artificial uses. DO_NOT_GEN is updated so that REG_DEAD notes are
>> @@ -3729,7 +3755,6 @@ df_set_unused_notes_for_mw (rtx insn, rt
>> bitmap live, bitmap do_not_gen,
>> bitmap artificial_uses)
>> {
>> - bool all_dead = true;
>> unsigned int r;
>>
>> #ifdef REG_DEAD_DEBUGGING
>> @@ -3737,18 +3762,11 @@ df_set_unused_notes_for_mw (rtx insn, rt
>> fprintf (dump_file, "mw_set_unused looking at mws[%d..%d]\n",
>> mws->start_regno, mws->end_regno);
>> #endif
>> - for (r=mws->start_regno; r <= mws->end_regno; r++)
>> - if ((bitmap_bit_p (live, r))
>> - || bitmap_bit_p (artificial_uses, r))
>> - {
>> - all_dead = false;
>> - break;
>> - }
>> -
>> - if (all_dead)
>> +
>> + if (df_whole_mw_reg_unused_p (mws, live, artificial_uses))
>> {
>> unsigned int regno = mws->start_regno;
>> - old = df_set_note (REG_UNUSED, insn, old, *(mws->loc));
>> + old = df_set_note (REG_UNUSED, insn, old, mws->mw_reg);
>>
>> #ifdef REG_DEAD_DEBUGGING
>> df_print_note ("adding 1: ", insn, REG_NOTES (insn));
>> @@ -3773,6 +3791,34 @@ df_set_unused_notes_for_mw (rtx insn, rt
>> }
>>
>>
>> +/* A subroutine of df_set_dead_notes_for_mw, with a selection of its
>> + arguments. Return true if the register value described by MWS's
>> + mw_reg is known to be completely dead, and if mw_reg can therefore
>> + be used in a REG_DEAD note. */
>> +
>> +static bool
>> +df_whole_mw_reg_dead_p (struct df_mw_hardreg *mws,
>> + bitmap live, bitmap artificial_uses,
>> + bitmap do_not_gen)
>> +{
>> + unsigned int r;
>> +
>> + /* If MWS describes a partial reference, create REG_DEAD notes for
>> + individual hard registers. */
>> + if (mws->flags & DF_REF_PARTIAL)
>> + return false;
>> +
>> + /* Likewise if some part of the register is not dead. */
>> + for (r = mws->start_regno; r <= mws->end_regno; r++)
>> + if (bitmap_bit_p (live, r)
>> + || bitmap_bit_p (artificial_uses, r)
>> + || bitmap_bit_p (do_not_gen, r))
>> + return false;
>> +
>> + gcc_assert (REG_P (mws->mw_reg));
>> + return true;
>> +}
>> +
>> /* Set the REG_DEAD notes for the multiword hardreg use in INSN based
>> on the bits in LIVE. DO_NOT_GEN is used to keep REG_DEAD notes
>> from being set if the instruction both reads and writes the
>> @@ -3783,7 +3829,6 @@ df_set_dead_notes_for_mw (rtx insn, rtx
>> bitmap live, bitmap do_not_gen,
>> bitmap artificial_uses)
>> {
>> - bool all_dead = true;
>> unsigned int r;
>>
>> #ifdef REG_DEAD_DEBUGGING
>> @@ -3799,25 +3844,13 @@ df_set_dead_notes_for_mw (rtx insn, rtx
>> }
>> #endif
>>
>> - for (r = mws->start_regno; r <= mws->end_regno; r++)
>> - if ((bitmap_bit_p (live, r))
>> - || bitmap_bit_p (artificial_uses, r)
>> - || bitmap_bit_p (do_not_gen, r))
>> - {
>> - all_dead = false;
>> - break;
>> - }
>> -
>> - if (all_dead)
>> + if (df_whole_mw_reg_dead_p (mws, live, artificial_uses, do_not_gen))
>> {
>> - if (!bitmap_bit_p (do_not_gen, mws->start_regno))
>> - {
>> - /* Add a dead note for the entire multi word register. */
>> - old = df_set_note (REG_DEAD, insn, old, *(mws->loc));
>> + /* Add a dead note for the entire multi word register. */
>> + old = df_set_note (REG_DEAD, insn, old, mws->mw_reg);
>> #ifdef REG_DEAD_DEBUGGING
>> - df_print_note ("adding 1: ", insn, REG_NOTES (insn));
>> + df_print_note ("adding 1: ", insn, REG_NOTES (insn));
>> #endif
>> - }
>> }
>> else
>> {
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: RFA: Stop df from generating SUBREG REG_NOTES
2007-07-13 10:15 ` RFA: Stop df from generating SUBREG REG_NOTES Richard Sandiford
2007-07-25 22:50 ` Richard Sandiford
@ 2007-07-27 0:01 ` Ian Lance Taylor
1 sibling, 0 replies; 28+ messages in thread
From: Ian Lance Taylor @ 2007-07-27 0:01 UTC (permalink / raw)
To: Richard Sandiford; +Cc: gcc-patches
Richard Sandiford <rsandifo@nildram.co.uk> writes:
> > gcc/
> > * df.h (df_mw_hardreg): Remove "loc" field.
> > * df-scan.c (df_ref_record): Don't set it. Remove redundant
> > local variable.
> > * df-problems.c (df_whole_mw_reg_unused_p): New function,
> > split out from df_set_unused_notes_for_mw. Return false for
> > partial references. Assert that mw_reg is a REG when returning true.
> > (df_set_unused_notes_for_mw): Use it. Use mw_reg instead of *loc.
> > (df_whole_mw_reg_dead_p): New function, split out from
> > df_set_dead_notes_for_mw. Return false for partial references.
> > Assert that mw_reg is a REG when returning true.
> > (df_set_dead_notes_for_mw): Use it. Use mw_reg instead of *loc.
> > Remove redundant bitmap check.
This is OK.
Thanks.
Ian
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2007-07-26 23:35 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-06-26 3:43 RTL sharing tester (for testing) Jan Hubicka
2007-06-27 15:55 ` Kenneth Zadeck
2007-06-27 18:17 ` Diego Novillo
2007-06-27 21:43 ` Jan Hubicka
2007-06-28 2:02 ` Kaz Kojima
2007-06-28 4:24 ` Jan Hubicka
2007-06-28 8:11 ` Paolo Bonzini
2007-06-28 17:16 ` Kenneth Zadeck
2007-06-28 18:26 ` Jan Hubicka
2007-06-28 19:57 ` Kenneth Zadeck
2007-06-28 20:27 ` Richard Sandiford
2007-06-29 8:29 ` Richard Sandiford
2007-06-29 12:11 ` Kenneth Zadeck
2007-06-29 13:35 ` Paolo Bonzini
2007-06-29 13:37 ` Bernd Schmidt
2007-06-29 13:39 ` Jan Hubicka
2007-06-29 16:55 ` Richard Sandiford
2007-07-13 10:15 ` RFA: Stop df from generating SUBREG REG_NOTES Richard Sandiford
2007-07-25 22:50 ` Richard Sandiford
2007-07-27 0:01 ` Ian Lance Taylor
2007-06-28 20:19 ` RTL sharing tester (for testing) Paolo Bonzini
2007-06-27 23:06 ` Mark Mitchell
2007-06-27 23:06 ` Jan Hubicka
2007-06-27 23:12 ` Eric Christopher
2007-06-28 0:48 ` Jan Hubicka
2007-06-28 22:24 ` Eric Christopher
2007-06-29 23:14 ` Graham Stott
2007-06-30 0:17 ` Jan Hubicka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).