[Bug debug/43092] New: [4.5 Regression] Wrong debuginfo with VTA and -fomit-frame-pointer/-mno-accumulate-outgoing-args

[Bug debug/43092] New: [4.5 Regression] Wrong debuginfo with VTA and -fomit-frame-pointer/-mno-accumulate-outgoing-args

[jakub at gcc dot gnu dot org]

/* { 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.


-- 
           Summary: [4.5 Regression] Wrong debuginfo with VTA and -fomit-
                    frame-pointer/-mno-accumulate-outgoing-args
           Product: gcc
           Version: 4.5.0
            Status: UNCONFIRMED
          Keywords: wrong-debug
          Severity: normal
          Priority: P3
         Component: debug
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: jakub at gcc dot gnu dot org
GCC target triplet: i686-linux


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43092

[Bug debug/43092] [4.5 Regression] Wrong debuginfo with VTA and -fomit-frame-pointer/-mno-accumulate-outgoing-args

[jakub at gcc dot gnu dot org]

-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
   Target Milestone|---                         |4.5.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43092

[Bug debug/43092] [4.5 Regression] Wrong debuginfo with VTA and -fomit-frame-pointer/-mno-accumulate-outgoing-args

[jakub at gcc dot gnu dot org]

------- Comment #1 from jakub at gcc dot gnu dot org  2010-02-16 15:17 -------
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.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43092

[Bug debug/43092] [4.5 Regression] Wrong debuginfo with VTA and -fomit-frame-pointer/-mno-accumulate-outgoing-args

[jakub at gcc dot gnu dot org]

------- Comment #2 from jakub at gcc dot gnu dot org  2010-02-16 16:27 -------
Created an attachment (id=19890)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=19890&action=view)
patch

Incomplete patch I've tried, didn't make any difference unfortunately.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43092

[Bug debug/43092] [4.5 Regression] Wrong debuginfo with VTA and -fomit-frame-pointer/-mno-accumulate-outgoing-args

[jakub at gcc dot gnu dot org]

------- Comment #3 from jakub at gcc dot gnu dot org  2010-02-16 19:45 -------
Created an attachment (id=19891)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=19891&action=view)
gcc45-pr43092.patch

Different patch where the replacement is done when emitting notes, not early.
This seems to cure the testcase.


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #19890|0                           |1
        is obsolete|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43092

[Bug debug/43092] [4.5 Regression] Wrong debuginfo with VTA and -fomit-frame-pointer/-mno-accumulate-outgoing-args

[jakub at gcc dot gnu dot org]

------- Comment #4 from jakub at gcc dot gnu dot org  2010-03-16 10:51 -------
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


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43092

[Bug debug/43092] [4.5 Regression] Wrong debuginfo with VTA and -fomit-frame-pointer/-mno-accumulate-outgoing-args

[jakub at gcc dot gnu dot org]

------- Comment #5 from jakub at gcc dot gnu dot org  2010-03-16 10:53 -------
Fixed.


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43092

[Bug debug/43092] [4.5 Regression] Wrong debuginfo with VTA and -fomit-frame-pointer/-mno-accumulate-outgoing-args

[aoliva at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43092

--- Comment #6 from Alexandre Oliva <aoliva at gcc dot gnu.org> 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