[commit, spu] Fix regression (Re: [PR debug/47590] rework md option overriding to delay var-tracking)
Alexandre Oliva wrote:
* config/spu/spu.c (spu_flag_var_tracking): Drop.
(TARGET_DELAY_VARTRACK): Define.
(spu_var_tracking): New.
(spu_machine_dependent_reorg): Call it.
(asm_file_start): Don't save and override flag_var_tracking.
This change caused crashes under certain circumstances. The problem
is that spu_var_tracking calls df_analyze, which assumes the df
framework has been keeping up to date on instructions. In particular,
it assumes that df_scan_insn was called for each insn that was
generated in the meantime. Normally, this is not a problem, because
the emit_insn family itself calls df_scan_insn.
However, this works only as long as the assignment of insns to basic
blocks is valid. In machine-dependent reorg, this is no longer true.
To fix this, the current place in spu_machine_dependent_reorg that
used to call df_analyze made sure to re-install the basic-block
mappings by calling compute_bb_for_insn before.
The new location where your patch has added a call to df_analyze,
however, is out of the scope of that existing compute_bb_for_insn
call, causing those problems.
Fixed by adding another compute_bb_for_insn/free_bb_for_insn pair
to cover the new call site as well.
Also, asm_file_start now no longer does anything interesting,
and can just be removed in favor of the default implementation.
Tested on spu-elf, committed to mainline.
Bye,
Ulrich
ChangeLog:
* config/spu/spu.c (TARGET_ASM_FILE_START): Do not define.
(asm_file_start): Remove.
(spu_machine_dependent_reorg): Call compute_bb_for_insn and
free_bb_for_insn around code that modifies insns before
restarting df analysis.
Index: gcc/config/spu/spu.c
===
*** gcc/config/spu/spu.c(revision 176209)
--- gcc/config/spu/spu.c(working copy)
*** static enum machine_mode spu_addr_space_
*** 224,230
static bool spu_addr_space_subset_p (addr_space_t, addr_space_t);
static rtx spu_addr_space_convert (rtx, tree, tree);
static int spu_sms_res_mii (struct ddg *g);
- static void asm_file_start (void);
static unsigned int spu_section_type_flags (tree, const char *, int);
static section *spu_select_section (tree, int, unsigned HOST_WIDE_INT);
static void spu_unique_section (tree, int);
--- 224,229
*** static void spu_setup_incoming_varargs (
*** 462,470
#undef TARGET_SCHED_SMS_RES_MII
#define TARGET_SCHED_SMS_RES_MII spu_sms_res_mii
- #undef TARGET_ASM_FILE_START
- #define TARGET_ASM_FILE_START asm_file_start
-
#undef TARGET_SECTION_TYPE_FLAGS
#define TARGET_SECTION_TYPE_FLAGS spu_section_type_flags
--- 461,466
*** spu_machine_dependent_reorg (void)
*** 2703,2711
--- 2699,2709
{
/* We still do it for unoptimized code because an external
function might have hinted a call or return. */
+ compute_bb_for_insn ();
insert_hbrp ();
pad_bb ();
spu_var_tracking ();
+ free_bb_for_insn ();
return;
}
*** spu_libgcc_shift_count_mode (void)
*** 7039,7052
return SImode;
}
- /* An early place to adjust some flags after GCC has finished processing
- * them. */
- static void
- asm_file_start (void)
- {
- default_file_start ();
- }
-
/* Implement targetm.section_type_flags. */
static unsigned int
spu_section_type_flags (tree decl, const char *name, int reloc)
--- 7037,7042
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
ulrich.weig...@de.ibm.com
Re: [PR debug/47590] rework md option overriding to delay var-tracking
On Jun 2, 2011, Alexandre Oliva aol...@redhat.com wrote: Right you are, though it looks like leaving the @hook lines out makes no difference. Anyhow, here's the patch I'm checking in. Trunk is now fixed, is the patch ok for the 4.6 branch too? for gcc/ChangeLog from Alexandre Oliva aol...@redhat.com PR debug/47590 * target.def (delay_sched2, delay_vartrack): New. * doc/tm.texi.in: Update. * doc/tm.texi: Rebuild. * sched-rgn.c (gate_handle_sched2): Fail if delay_sched2. * var-tracking.c (gate_handle_var_tracking): Likewise. * config/bfin/bfin.c (bfin_flag_schedule_insns2): Drop. (bfin_flag_var_tracking): Drop. (output_file_start): Don't save and override flag_var_tracking. (bfin_option_override): Ditto flag_schedule_insns_after_reload. (bfin_reorg): Test original variables. (TARGET_DELAY_SCHED2, TARGET_DELAY_VARTRACK): Define. * config/ia64/ia64.c (ia64_flag_schedule_insns2): Drop. (ia64_flag_var_tracking): Drop. (TARGET_DELAY_SCHED2, TARGET_DELAY_VARTRACK): Define. (ia64_file_start): Don't save and override flag_var_tracking. (ia64_override_options_after_change): Ditto flag_schedule_insns_after_reload. (ia64_reorg): Test original variables. * config/picochip/picochip.c (picochip_flag_schedule_insns2): Drop. (picochip_flag_var_tracking): Drop. (TARGET_DELAY_SCHED2, TARGET_DELAY_VARTRACK): Define. (picochip_option_override): Don't save and override flag_schedule_insns_after_reload. (picochip_asm_file_start): Ditto flag_var_tracking. (picochip_reorg): Test original variables. * config/spu/spu.c (spu_flag_var_tracking): Drop. (TARGET_DELAY_VARTRACK): Define. (spu_var_tracking): New. (spu_machine_dependent_reorg): Call it. (asm_file_start): Don't save and override flag_var_tracking. -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist Red Hat Brazil Compiler Engineer
Re: [PR debug/47590] rework md option overriding to delay var-tracking
On 06/03/2011 04:11 PM, Alexandre Oliva wrote: Trunk is now fixed, is the patch ok for the 4.6 branch too? As far as I'm concerned, sure. Bernd
Re: [PR debug/47590] rework md option overriding to delay var-tracking
On Jun 1, 2011, Bernd Schmidt ber...@codesourcery.com wrote:
On 06/02/2011 12:47 AM, Alexandre Oliva wrote:
On Jun 1, 2011, Bernd Schmidt ber...@codesourcery.com wrote:
Looks ok, except I think you need to update tm.texi.in and tm.texi?
Oh, I didn't realize updating tm.texi.in; AFAICT tm.texi is generated
the same regardless.
I *think* what one is supposed to do is to just add the @hook lines in
tm.texi.in if the definition in target.def includes documentation.
Right you are, though it looks like leaving the @hook lines out makes no
difference. Anyhow, here's the patch I'm checking in.
for gcc/ChangeLog
from Alexandre Oliva aol...@redhat.com
PR debug/47590
* target.def (delay_sched2, delay_vartrack): New.
* doc/tm.texi.in: Update.
* doc/tm.texi: Rebuild.
* sched-rgn.c (gate_handle_sched2): Fail if delay_sched2.
* var-tracking.c (gate_handle_var_tracking): Likewise.
* config/bfin/bfin.c (bfin_flag_schedule_insns2): Drop.
(bfin_flag_var_tracking): Drop.
(output_file_start): Don't save and override flag_var_tracking.
(bfin_option_override): Ditto flag_schedule_insns_after_reload.
(bfin_reorg): Test original variables.
(TARGET_DELAY_SCHED2, TARGET_DELAY_VARTRACK): Define.
* config/ia64/ia64.c (ia64_flag_schedule_insns2): Drop.
(ia64_flag_var_tracking): Drop.
(TARGET_DELAY_SCHED2, TARGET_DELAY_VARTRACK): Define.
(ia64_file_start): Don't save and override flag_var_tracking.
(ia64_override_options_after_change): Ditto
flag_schedule_insns_after_reload.
(ia64_reorg): Test original variables.
* config/picochip/picochip.c (picochip_flag_schedule_insns2): Drop.
(picochip_flag_var_tracking): Drop.
(TARGET_DELAY_SCHED2, TARGET_DELAY_VARTRACK): Define.
(picochip_option_override): Don't save and override
flag_schedule_insns_after_reload.
(picochip_asm_file_start): Ditto flag_var_tracking.
(picochip_reorg): Test original variables.
* config/spu/spu.c (spu_flag_var_tracking): Drop.
(TARGET_DELAY_VARTRACK): Define.
(spu_var_tracking): New.
(spu_machine_dependent_reorg): Call it.
(asm_file_start): Don't save and override flag_var_tracking.
Index: gcc/target.def
===
--- gcc/target.def.orig 2011-05-30 03:53:29.0 -0300
+++ gcc/target.def 2011-05-31 17:50:09.733284971 -0300
@@ -2717,6 +2717,16 @@ DEFHOOKPOD
in particular GDB does not use them.,
bool, false)
+DEFHOOKPOD
+(delay_sched2, True if sched2 is not to be run at its normal place. \
+This usually means it will be run as part of machine-specific reorg.,
+bool, false)
+
+DEFHOOKPOD
+(delay_vartrack, True if vartrack is not to be run at its normal place. \
+This usually means it will be run as part of machine-specific reorg.,
+bool, false)
+
/* Leave the boolean fields at the end. */
/* Close the 'struct gcc_target' definition. */
Index: gcc/doc/tm.texi.in
===
--- gcc/doc/tm.texi.in.orig 2011-06-01 19:45:28.725386885 -0300
+++ gcc/doc/tm.texi.in 2011-06-01 19:53:47.394534907 -0300
@@ -9353,6 +9353,10 @@ tables, and hence is desirable if it wor
@hook TARGET_WANT_DEBUG_PUB_SECTIONS
+@hook TARGET_DELAY_SCHED2
+
+@hook TARGET_DELAY_VARTRACK
+
@defmac ASM_OUTPUT_DWARF_DELTA (@var{stream}, @var{size}, @var{label1}, @var{label2})
A C statement to issue assembly directives that create a difference
@var{lab1} minus @var{lab2}, using an integer of the given @var{size}.
Index: gcc/doc/tm.texi
===
--- gcc/doc/tm.texi.orig 2011-05-30 03:53:29.0 -0300
+++ gcc/doc/tm.texi 2011-06-01 19:54:09.126494927 -0300
@@ -9432,6 +9432,14 @@ tables, and hence is desirable if it wor
True if the @code{.debug_pubtypes} and @code{.debug_pubnames} sections should be emitted. These sections are not used on most platforms, and in particular GDB does not use them.
@end deftypevr
+@deftypevr {Target Hook} bool TARGET_DELAY_SCHED2
+True if sched2 is not to be run at its normal place. This usually means it will be run as part of machine-specific reorg.
+@end deftypevr
+
+@deftypevr {Target Hook} bool TARGET_DELAY_VARTRACK
+True if vartrack is not to be run at its normal place. This usually means it will be run as part of machine-specific reorg.
+@end deftypevr
+
@defmac ASM_OUTPUT_DWARF_DELTA (@var{stream}, @var{size}, @var{label1}, @var{label2})
A C statement to issue assembly directives that create a difference
@var{lab1} minus @var{lab2}, using an integer of the given @var{size}.
Index: gcc/sched-rgn.c
===
--- gcc/sched-rgn.c.orig 2011-04-06 00:24:12.0 -0300
+++ gcc/sched-rgn.c 2011-05-31 17:43:02.584808465 -0300
@@ -3508,7 +3508,7 @@ gate_handle_sched2 (void)
{
#ifdef INSN_SCHEDULING
return optimize 0 flag_schedule_insns_after_reload
- dbg_cnt (sched2_func);
+ !targetm.delay_sched2 dbg_cnt (sched2_func);
#else
return 0;
#endif
Re: [PR debug/47590] rework md option overriding to delay var-tracking
On May 4, 2011, Bernd Schmidt ber...@codesourcery.com wrote:
This comment looks very weird when added to ia64_option_override
(likewise for other targets). Is there a reason it's not true anymore?
Dunno, but the patch definitely didn't work any more when I retested it.
Maybe it didin't work when I first tested it, I don't recall having
actually looked at debug dumps then, but it was a while ago.
Here's an alternate approach that works now. Regstrapped on
32- and 64-bit x86-linux-gnu, and cross-built on x86_64-linux-gnu to the
4 affected platforms, checking manually the proper presence of debug
insns and location notes at the expected dump files.
Ok to install?
for gcc/ChangeLog
from Alexandre Oliva aol...@redhat.com
PR debug/47590
* config/bfin/bfin.c (output_file_start): Move flag_var_tracking
overriding...
(bfin_option_override): ... here.
* config/ia64/ia64.c (ia64_file_start): Likewise...
(ia64_option_override): ... ditto.
* config/spu/spu.c (asm_file_start): Likewise...
(spu_option_override): ... ditto.
* config/picochip/picochip.c (picochip_asm_file_start): Likewise...
(picochip_option_override): ... ditto. Split previous code into...
(picochip_override_options_after_change): ... this new function.
(TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE): Use the latter.
Index: gcc/config/bfin/bfin.c
===
--- gcc/config/bfin/bfin.c.orig 2011-05-31 12:51:54.361391409 -0300
+++ gcc/config/bfin/bfin.c 2011-05-31 17:57:54.264722118 -0300
@@ -86,14 +86,6 @@ const char *byte_reg_names[] = BYTE_R
static int arg_regs[] = FUNCTION_ARG_REGISTERS;
static int ret_regs[] = FUNCTION_RETURN_REGISTERS;
-/* Nonzero if -fschedule-insns2 was given. We override it and
- call the scheduler ourselves during reorg. */
-static int bfin_flag_schedule_insns2;
-
-/* Determines whether we run variable tracking in machine dependent
- reorganization. */
-static int bfin_flag_var_tracking;
-
struct bfin_cpu
{
const char *name;
@@ -375,13 +367,6 @@ output_file_start (void)
FILE *file = asm_out_file;
int i;
- /* Variable tracking should be run after all optimizations which change order
- of insns. It also needs a valid CFG. This can't be done in
- bfin_option_override, because flag_var_tracking is finalized after
- that. */
- bfin_flag_var_tracking = flag_var_tracking;
- flag_var_tracking = 0;
-
fprintf (file, .file \%s\;\n, input_filename);
for (i = 0; arg_regs[i] = 0; i++)
@@ -2772,11 +2757,6 @@ bfin_option_override (void)
flag_schedule_insns = 0;
- /* Passes after sched2 can break the helpful TImode annotations that
- haifa-sched puts on every insn. Just do scheduling in reorg. */
- bfin_flag_schedule_insns2 = flag_schedule_insns_after_reload;
- flag_schedule_insns_after_reload = 0;
-
init_machine_status = bfin_init_machine_status;
}
@@ -5550,7 +5530,7 @@ bfin_reorg (void)
with old MDEP_REORGS that are not CFG based. Recompute it now. */
compute_bb_for_insn ();
- if (bfin_flag_schedule_insns2)
+ if (flag_schedule_insns_after_reload)
{
splitting_for_sched = 1;
split_all_insns ();
@@ -5579,7 +5559,7 @@ bfin_reorg (void)
workaround_speculation ();
- if (bfin_flag_var_tracking)
+ if (flag_var_tracking)
{
timevar_push (TV_VAR_TRACKING);
variable_tracking_main ();
@@ -6765,4 +6745,14 @@ bfin_conditional_register_usage (void)
#undef TARGET_EXTRA_LIVE_ON_ENTRY
#define TARGET_EXTRA_LIVE_ON_ENTRY bfin_extra_live_on_entry
+/* Passes after sched2 can break the helpful TImode annotations that
+ haifa-sched puts on every insn. Just do scheduling in reorg. */
+#undef TARGET_DELAY_SCHED2
+#define TARGET_DELAY_SCHED2 true
+
+/* Variable tracking should be run after all optimizations which
+ change order of insns. It also needs a valid CFG. */
+#undef TARGET_DELAY_VARTRACK
+#define TARGET_DELAY_VARTRACK true
+
struct gcc_target targetm = TARGET_INITIALIZER;
Index: gcc/config/ia64/ia64.c
===
--- gcc/config/ia64/ia64.c.orig 2011-05-31 12:51:54.370391376 -0300
+++ gcc/config/ia64/ia64.c 2011-05-31 17:55:56.30254 -0300
@@ -103,14 +103,6 @@ static const char * const ia64_local_reg
static const char * const ia64_output_reg_names[8] =
{ out0, out1, out2, out3, out4, out5, out6, out7 };
-/* Determines whether we run our final scheduling pass or not. We always
- avoid the normal second scheduling pass. */
-static int ia64_flag_schedule_insns2;
-
-/* Determines whether we run variable tracking in machine dependent
- reorganization. */
-static int ia64_flag_var_tracking;
-
/* Variables which are this size or smaller are put in the sdata/sbss
sections. */
@@ -640,6 +632,14 @@ static const struct default_options ia64
#undef TARGET_PREFERRED_RELOAD_CLASS
#define TARGET_PREFERRED_RELOAD_CLASS ia64_preferred_reload_class
+#undef TARGET_DELAY_SCHED2
Re: [PR debug/47590] rework md option overriding to delay var-tracking
On 06/01/2011 10:10 PM, Alexandre Oliva wrote: On May 4, 2011, Bernd Schmidt ber...@codesourcery.com wrote: This comment looks very weird when added to ia64_option_override (likewise for other targets). Is there a reason it's not true anymore? Dunno, but the patch definitely didn't work any more when I retested it. Maybe it didin't work when I first tested it, I don't recall having actually looked at debug dumps then, but it was a while ago. Here's an alternate approach that works now. Regstrapped on 32- and 64-bit x86-linux-gnu, and cross-built on x86_64-linux-gnu to the 4 affected platforms, checking manually the proper presence of debug insns and location notes at the expected dump files. Ok to install? Looks ok, except I think you need to update tm.texi.in and tm.texi? Bernd
Re: [PR debug/47590] rework md option overriding to delay var-tracking
On Jun 1, 2011, Bernd Schmidt ber...@codesourcery.com wrote:
On 06/01/2011 10:10 PM, Alexandre Oliva wrote:
On May 4, 2011, Bernd Schmidt ber...@codesourcery.com wrote:
This comment looks very weird when added to ia64_option_override
(likewise for other targets). Is there a reason it's not true anymore?
Dunno, but the patch definitely didn't work any more when I retested it.
Maybe it didin't work when I first tested it, I don't recall having
actually looked at debug dumps then, but it was a while ago.
Here's an alternate approach that works now. Regstrapped on
32- and 64-bit x86-linux-gnu, and cross-built on x86_64-linux-gnu to the
4 affected platforms, checking manually the proper presence of debug
insns and location notes at the expected dump files.
Ok to install?
Looks ok, except I think you need to update tm.texi.in and tm.texi?
Oh, I didn't realize updating tm.texi.in; AFAICT tm.texi is generated
the same regardless. Anyway, here's a patch with reordered hunks, a
proper ChangeLog (I failed to update it before posting) and with the
changes for tm.texi propagated to tm.texi.in.
I'll check it in soon if there aren't objections.
Thanks,
for gcc/ChangeLog
from Alexandre Oliva aol...@redhat.com
PR debug/47590
* target.def (delay_sched2, delay_vartrack): New.
* doc/tm.texi.in: Update.
* doc/tm.texi: Rebuild.
* sched-rgn.c (gate_handle_sched2): Fail if delay_sched2.
* var-tracking.c (gate_handle_var_tracking): Likewise.
* config/bfin/bfin.c (bfin_flag_schedule_insns2): Drop.
(bfin_flag_var_tracking): Drop.
(output_file_start): Don't save and override flag_var_tracking.
(bfin_option_override): Ditto flag_schedule_insns_after_reload.
(bfin_reorg): Test original variables.
(TARGET_DELAY_SCHED2, TARGET_DELAY_VARTRACK): Define.
* config/ia64/ia64.c (ia64_flag_schedule_insns2): Drop.
(ia64_flag_var_tracking): Drop.
(TARGET_DELAY_SCHED2, TARGET_DELAY_VARTRACK): Define.
(ia64_file_start): Don't save and override flag_var_tracking.
(ia64_override_options_after_change): Ditto
flag_schedule_insns_after_reload.
(ia64_reorg): Test original variables.
* config/picochip/picochip.c (picochip_flag_schedule_insns2): Drop.
(picochip_flag_var_tracking): Drop.
(TARGET_DELAY_SCHED2, TARGET_DELAY_VARTRACK): Define.
(picochip_option_override): Don't save and override
flag_schedule_insns_after_reload.
(picochip_asm_file_start): Ditto flag_var_tracking.
(picochip_reorg): Test original variables.
* config/spu/spu.c (spu_flag_var_tracking): Drop.
(TARGET_DELAY_VARTRACK): Define.
(spu_var_tracking): New.
(spu_machine_dependent_reorg): Call it.
(asm_file_start): Don't save and override flag_var_tracking.
Index: gcc/target.def
===
--- gcc/target.def.orig 2011-05-30 03:53:29.0 -0300
+++ gcc/target.def 2011-05-31 17:50:09.733284971 -0300
@@ -2717,6 +2717,16 @@ DEFHOOKPOD
in particular GDB does not use them.,
bool, false)
+DEFHOOKPOD
+(delay_sched2, True if sched2 is not to be run at its normal place. \
+This usually means it will be run as part of machine-specific reorg.,
+bool, false)
+
+DEFHOOKPOD
+(delay_vartrack, True if vartrack is not to be run at its normal place. \
+This usually means it will be run as part of machine-specific reorg.,
+bool, false)
+
/* Leave the boolean fields at the end. */
/* Close the 'struct gcc_target' definition. */
Index: gcc/doc/tm.texi.in
===
--- gcc/doc/tm.texi.in.orig 2011-06-01 19:45:28.725386885 -0300
+++ gcc/doc/tm.texi.in 2011-06-01 19:46:20.062304033 -0300
@@ -9352,6 +9352,20 @@ tables, and hence is desirable if it wor
@end defmac
@hook TARGET_WANT_DEBUG_PUB_SECTIONS
+True if the @code{.debug_pubtypes} and @code{.debug_pubnames} sections
+should be emitted. These sections are not used on most platforms, and
+in particular GDB does not use them.
+@end deftypevr
+
+@hook TARGET_DELAY_SCHED2
+True if sched2 is not to be run at its normal place. This usually means
+it will be run as part of machine-specific reorg.
+@end deftypevr
+
+@hook TARGET_DELAY_VARTRACK
+True if vartrack is not to be run at its normal place. This usually
+means it will be run as part of machine-specific reorg.
+@end deftypevr
@defmac ASM_OUTPUT_DWARF_DELTA (@var{stream}, @var{size}, @var{label1}, @var{label2})
A C statement to issue assembly directives that create a difference
Index: gcc/doc/tm.texi
===
--- gcc/doc/tm.texi.orig 2011-05-30 03:53:29.0 -0300
+++ gcc/doc/tm.texi 2011-05-31 17:52:19.841837811 -0300
@@ -9432,6 +9432,14 @@ tables, and hence is desirable if it wor
True if the @code{.debug_pubtypes} and @code{.debug_pubnames} sections should be emitted. These sections are not used on most platforms, and in particular GDB does not use them.
@end deftypevr
+@deftypevr {Target Hook} bool TARGET_DELAY_SCHED2
+True if sched2 is
Re: [PR debug/47590] rework md option overriding to delay var-tracking
On 04/02/2011 10:15 AM, Alexandre Oliva wrote: Some targets delayed the var-tracking pass to run it after machine-specific transformations. The introduction of option saving and restoring broke this, because the machine-specific overriding took place too late for it to be saved, so, after compiling a function that used a different set of options, we'd restore incorrect flags, running var-tracking at the wrong time. This patch fixes the handling of this option so that it takes place at the right time. It does not, however, support per-function overriding of -fvar-tracking; I'm not sure how to implement that with the current framework. Suggestions? @@ -5722,6 +5715,13 @@ ia64_option_override (void) if (TARGET_ABI_OPEN_VMS) flag_no_common = 1; + /* Variable tracking should be run after all optimizations which change order + of insns. It also needs a valid CFG. This can't be done in + ia64_option_override, because flag_var_tracking is finalized after + that. */ This comment looks very weird when added to ia64_option_override (likewise for other targets). Is there a reason it's not true anymore? Bernd
