Bug 43092 - [4.5 Regression] Wrong debuginfo with VTA and -fomit-frame-pointer/-mno-accumulate-outgoing-args
[4.5 Regression] Wrong debuginfo with VTA and -fomit-frame-pointer/-mno-accum...
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: debug
4.5.0
: P1 normal
: 4.5.0
Assigned To: Not yet assigned to anyone
: wrong-debug
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-02-16 13:25 UTC by Jakub Jelinek
Modified: 2011-02-03 06:04 UTC (History)
2 users (show)

See Also:
Host:
Target: i686-linux
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
patch (3.28 KB, patch)
2010-02-16 16:27 UTC, Jakub Jelinek
Details | Diff
gcc45-pr43092.patch (6.61 KB, patch)
2010-02-16 19:45 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2010-02-16 13:25:04 UTC
/* { dg-do run } */
/* { dg-options "-O2 -march=i586 -dA -g -fomit-frame-pointer" { target { { i?86-*-* x86_64-*-* } && ilp32 } } } */

void __attribute__((noinline))
bar (int *p)
{
  (*p)++;
  asm volatile ("" : "=r" (*p) : "0" (*p) : "memory");
}

void __attribute__((noinline))
baz (int a, int b, int c, int d, int e, int f)
{
  asm volatile ("" : : "r" (a + b + c + d + e + f) : "memory");
}

int __attribute__((noinline))
foo (int x)
{
  int y;
  y = 5;
  bar (&y);
  baz (0, 1, 2, 3, 4, 5);
  bar (&x);
  y++;
  baz (1, 2, 3, 4, 5, 6);
  bar (&y);
  return x + y;
}

int
main ()
{
  if (foo (17) != 8 + 18)
    __builtin_abort ();
  return 0;
}

has wrong debuginfo for variable y - (mem (sp + offset)) live across several sp adjustments.  In the debugger inside foo when using nexti and printing y after each nexti, it first starts as <optimized out> (expected), then y = 5 is executed and it prints 5, but then sp is adjusted and it prints garbage because sp changed.

I believe we should be de-eliminating stack_pointer_rtx resp. hard_frame_pointer_rtx based MEMs in var-tracking generated notes back to arg_pointer_rtx / frame_pointer_rtx based notes when they aren't meant to change whenever sp resp. fp changes.

On the http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43051#c1 testcase this doesn't show up as wrong debug, but just inefficient location list (LVL1 through LVL2 the argument is fbreg 0, while from LVL2 to LVL12 it is breg5 8 when both
locations are actually the same memory.  There is also the additional problem that we leave out the epilogue ret insn, as if the parameter wasn't defined at that spot.
Comment 1 Jakub Jelinek 2010-02-16 15:17:08 UTC
For sp based addresses in the -fno-var-tracking-assignments case we have vt_stack_adjustments that replaces the sp based MEMs with CFA based ones (frame_pointer_rtx resp. arg_pointer_rtx based ones).
Unfortunately, add_uses replaces the addresses of the MEMs with VALUEs as they are REG + CONST_INT instead of just REG or MEM, and therefore vt_stack_adjustments does nothing.

Either add_uses should also special case REG + CONST_INT (or perhaps just sp/fp + CONST_INT), or we need to do the replacement from sp (and ideally also fp) afterwards.
Comment 2 Jakub Jelinek 2010-02-16 16:27:32 UTC
Created attachment 19890 [details]
patch

Incomplete patch I've tried, didn't make any difference unfortunately.
Comment 3 Jakub Jelinek 2010-02-16 19:45:06 UTC
Created attachment 19891 [details]
gcc45-pr43092.patch

Different patch where the replacement is done when emitting notes, not early.
This seems to cure the testcase.
Comment 4 Jakub Jelinek 2010-03-16 10:51:17 UTC
Subject: Bug 43092

Author: jakub
Date: Tue Mar 16 10:50:42 2010
New Revision: 157476

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157476
Log:
	PR debug/43051
	PR debug/43092
	* cselib.c (cselib_preserve_constants,
	cfa_base_preserved_val): New static variables.
	(preserve_only_constants): New function.
	(cselib_reset_table): If cfa_base_preserved_val is non-NULL, don't
	clear its REG_VALUES.  If cselib_preserve_constants, don't 
	empty the whole hash table, but preserve there VALUEs with constants,
	cfa_base_preserved_val and cfa_base_preserved_val plus constant.
	(cselib_preserve_cfa_base_value): New function.
	(cselib_invalidate_regno): Don't invalidate cfa_base_preserved_val.
	(cselib_init): Change argument to int bitfield.  Set
	cselib_preserve_constants to whether CSELIB_PRESERVE_CONSTANTS
	is in it.
	(cselib_finish): Clear cselib_preserve_constants and
	cfa_base_preserved_val.
	* cselib.h (enum cselib_record_what): New enum.
	(cselib_init): Change argument to int.
	(cselib_preserve_cfa_base_value): New prototype.
	* postreload.c (reload_cse_regs_1): Adjust cselib_init caller.
	* dse.c (dse_step1): Likewise.
	* cfgcleanup.c (thread_jump): Likewise.
	* sched-deps.c (sched_analyze): Likewise.
	* gcse.c (local_cprop_pass): Likewise.
	* simplify-rtx.c (simplify_replace_fn_rtx): Add argument to callback.
	If FN is non-NULL, call the callback always and whenever it returns
	non-NULL just return that.  Only do rtx_equal_p if FN is NULL.
	* rtl.h (simplify_replace_fn_rtx): Add argument to callback.
	* combine.c (propagate_for_debug_subst): Add old_rtx argument,
	compare from with old_rtx and if it isn't rtx_equal_p, return NULL.
	* Makefile.in (var-tracking.o): Depend on $(RECOG_H).
	* var-tracking.c: Include recog.h.
	(bb_stack_adjust_offset): Remove.
	(vt_stack_adjustments): Don't call it, instead just gather the
	adjustments using insn_stack_adjust_offset_pre_post on each bb insn.
	(adjust_stack_reference): Remove.
	(compute_cfa_pointer): New function.
	(hard_frame_pointer_adjustment, cfa_base_rtx): New static variables.
	(struct adjust_mem_data): New type.
	(adjust_mems, adjust_mem_uses, adjust_mem_stores, adjust_insn): New
	functions.
	(get_address_mode): New function.
	(replace_expr_with_values): Use it.
	(use_type): Don't do cselib_lookup for VAR_LOC_UNKNOWN_P.
	Use get_address_mode.  For cfa_base_rtx return MO_CLOBBER.
	(adjust_sets): Remove.
	(add_uses): Don't add extra MO_VAL_USE for cfa_base_rtx plus constant.
	Use get_address_mode.
	(get_adjusted_src): Remove.
	(add_stores): Don't call it.  Never reuse expr SET.  Don't add extra
	MO_VAL_USE for cfa_base_rtx plus constant.  Use get_address_mode.
	(add_with_sets): Don't call adjust_sets.
	(fp_setter, vt_init_cfa_base): New functions.
	(vt_initialize): Change return type to bool.  Move most of pool etc.
	initialization to the beginning of the function from end.  Pass
	CSELIB_RECORD_MEMORY | CSELIB_PRESERVE_CONSTANTS to cselib_init.
	If !frame_pointer_needed, call vt_stack_adjustment before mos
	vector is filled, call vt_init_cfa_base if argp/framep has been
	eliminated to sp.  If frame_pointer_needed and argp/framep has
	been eliminated to hard frame pointer, set
	hard_frame_pointer_adjustment and call vt_init_cfa_base after
	encountering fp setter in the prologue.  For MO_ADJUST, call
	log_op_type before pusing the op into mos vector, not afterwards.
	Call adjust_insn before cselib_process_insn/add_with_sets,
	call cancel_changes (0) afterwards.
	(variable_tracking_main_1): Adjust for vt_initialize calling
	vt_stack_adjustments and returning whether it succeeded or not.

	* gcc.dg/guality/pr43051-1.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/guality/pr43051-1.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/Makefile.in
    trunk/gcc/cfgcleanup.c
    trunk/gcc/combine.c
    trunk/gcc/cselib.c
    trunk/gcc/cselib.h
    trunk/gcc/dse.c
    trunk/gcc/gcse.c
    trunk/gcc/postreload.c
    trunk/gcc/rtl.h
    trunk/gcc/sched-deps.c
    trunk/gcc/simplify-rtx.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/var-tracking.c

Comment 5 Jakub Jelinek 2010-03-16 10:53:00 UTC
Fixed.
Comment 6 Alexandre Oliva 2011-02-03 06:04:07 UTC
Author: aoliva
Date: Thu Feb  3 06:04:04 2011
New Revision: 169782

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=169782
Log:
PR debug/43092
PR rtl-optimization/43494
* rtl.h (for_each_inc_dec_fn): New type.
(for_each_inc_dec): Declare.
* rtlanal.c (struct for_each_inc_dec_ops): New type.
(for_each_inc_dec_find_inc_dec): New fn.
(for_each_inc_dec_find_mem): New fn.
(for_each_inc_dec): New fn.
* dse.c (struct insn_size): Remove.
(replace_inc_dec, replace_inc_dec_mem): Remove.
(emit_inc_dec_insn_before): New fn.
(check_for_inc_dec): Use it, along with for_each_inc_dec.
(canon_address): Pass mem modes to cselib_lookup.
* cselib.h (cselib_lookup): Add memmode argument.  Adjust callers.
(cselib_lookup_from_insn): Likewise.
(cselib_subst_to_values): Likewise.
* cselib.c (find_slot_memmode): New var.
(cselib_find_slot): New fn.  Use it instead of
htab_find_slot_with_hash everywhere.
(entry_and_rtx_equal_p): Use find_slot_memmode.
(autoinc_split): New fn.
(rtx_equal_for_cselib_p): Rename and implement in terms of...
(rtx_equal_for_cselib_1): ... this.  Take memmode, pass it on.
Deal with autoinc.  Special-case recursion into MEMs.
(cselib_hash_rtx): Likewise.
(cselib_lookup_mem): Infer pmode from address mode.  Distinguish
address and MEM modes.
(cselib_subst_to_values): Add memmode, pass it on.
Deal with autoinc.
(cselib_lookup): Add memmode argument, pass it on.
(cselib_lookup_from_insn): Add memmode.
(cselib_invalidate_rtx): Discard obsolete push_operand handling.
(struct cselib_record_autoinc_data): New.
(cselib_record_autoinc_cb): New fn.
(cselib_record_sets): Use it, along with for_each_inc_dec.  Pass MEM
mode to cselib_lookup.  Reset autoinced REGs here instead of...
(cselib_process_insn): ... here.
* var-tracking.c (replace_expr_with_values, use_type): Pass MEM mode
to cselib_lookup.
(add_uses): Likewise, also to cselib_subst_to_values.
(add_stores): Likewise.
* sched-deps.c 	(add_insn_mem_dependence): Pass mode to
cselib_subst_to_values.
(sched_analyze_1, sched_analyze_2): Likewise.  Adjusted.
* gcse.c (do_local_cprop): Adjusted.
* postreload.c (reload_cse_simplify_set): Adjusted.
(reload_cse_simplify_operands): Adjusted.
* sel-sched-dump (debug_mem_addr_value): Pass mode.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cselib.c
    trunk/gcc/cselib.h
    trunk/gcc/dse.c
    trunk/gcc/gcse.c
    trunk/gcc/postreload.c
    trunk/gcc/rtl.h
    trunk/gcc/rtlanal.c
    trunk/gcc/sched-deps.c
    trunk/gcc/sel-sched-dump.c
    trunk/gcc/var-tracking.c