[Bug debug/86455] New: var-tracking mishandled pre_dec

vries at gcc dot gnu.org gcc-bugzilla@gcc.gnu.org
Tue Jul 10 11:19:00 GMT 2018


https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86455

            Bug ID: 86455
           Summary: var-tracking mishandled pre_dec
           Product: gcc
           Version: 9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: debug
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

With the experimental patch "[debug] Add -fkeep-vars-live" (PR78685 comment
12), there are two regressions of "Og -fkeep-vars-live" compared to "Og":
...
FAIL: gcc.dg/guality/csttest.c  -Og -fkeep-vars-live -DPREVENT_OPTIMIZATION  \
      line 29 n == -(0x17ULL << 50)
FAIL: gcc.dg/guality/csttest.c  -Og -fkeep-vars-live -DPREVENT_OPTIMIZATION  \
      line 55 j == -(0x17ULL << 31)
...

I've minimized one of the failures to:
...
/* PR debug/49676 */
/* { dg-do run { target lp64 } } */
/* { dg-options "-g -fno-ipa-icf" } */

volatile int v;

__attribute__((noinline, noclone))
unsigned long long
foo (unsigned long long x)
{
  unsigned long long c = x * (0x1ULL << 35);
  unsigned long long d = x * (0x17ULL << 15);
  unsigned long long e = x * (0x17ULL << 50);
  unsigned long long f = x * (0x37ULL << 31);
  unsigned long long g = x * (0x37ULL << 50);
  unsigned long long h = x * (0x1efULL << 33);
  unsigned long long i = x * (0x1efULL << 50);
  unsigned long long j = x * -(0x17ULL << 31);
  unsigned long long k = x * -(0x7ULL << 33);
  unsigned long long l = x * -(0x1ULL << 35);
  unsigned long long m = x * -(0x17ULL << 15);
  unsigned long long n = x * -(0x17ULL << 50);
  unsigned long long o = x * -(0x37ULL << 31);
  unsigned long long p = x * -(0x37ULL << 50);  /* { dg-final { gdb-test .+3
"p" "-(0x37ULL << 50)" } } */
  unsigned long long q = x * -(0x1efULL << 33);
  unsigned long long r = x * -(0x1efULL << 50);
  v++;
  return x;
}

__attribute__((noinline, noclone))
unsigned long long
bar (unsigned long long x)
{
  v++;
  return x;
}

int
main ()
{
  return foo (1) + bar (256) - 257;
}
...

The problem can be observed in the assembly:
...
        ## First, the imulq calculates variable p in %rcx.
        imulq   %rax, %rcx
        ## Immediately after that we save that value to stack at %rsp-8.
        movq    %rcx, -8(%rsp)
.LVL20:
        ## We let the debugger known that we can find p in %rcx.
        # DEBUG p => cx
        # csttest.c:25:3
        .loc 1 25 3 is_stmt 1
        # csttest.c:25:22
        .loc 1 25 22 is_stmt 0
        ## Here we reuse %rcx, so it no longer contains p.
        movabsq $-4252017623040, %rcx
.LVL21:
        ## And here we let the debugger know that p can be found at the stack
        ## location %rsp, which is incorrect, because it's at %rsp-8.
        # DEBUG p => [sp]
...

The problem is introduced by var-tracking.

Before var-tracking, we have:
...
(insn 126 71 127 2 (set (mem/c:DI (plus:DI (reg/f:DI 7 sp)
                (const_int -8 [0xfffffffffffffff8])) [2 %sfp+-8 S8 A64])
        (reg:DI 2 cx [127])) "csttest.c":24 85 {*movdi_internal}
     (expr_list:REG_DEAD (reg:DI 2 cx [127])
        (nil)))
(note 127 126 72 2 NOTE_INSN_DELETED)
(debug_insn 72 127 73 2 (var_location:DI p (mem/c:DI (plus:DI (reg/f:DI 7 sp)
            (const_int -8 [0xfffffffffffffff8])) [2 %sfp+-8 S8 A64]))
"csttest.c":24 -1
     (nil))
...
which is correct.

After var-tracking, we have:
...
(insn 126 71 127 2 (set (mem/c:DI (plus:DI (reg/f:DI 7 sp)
                (const_int -8 [0xfffffffffffffff8])) [2 %sfp+-8 S8 A64])
        (reg:DI 2 cx [127])) "csttest.c":24 85 {*movdi_internal}
     (expr_list:REG_DEAD (reg:DI 2 cx [127])
        (nil)))
(note 127 126 188 2 NOTE_INSN_DELETED)
(note 188 127 164 2 (var_location p (reg:DI 2 cx [127]))
NOTE_INSN_VAR_LOCATION)
(note 164 188 74 2 csttest.c:25 NOTE_INSN_BEGIN_STMT)
(insn 74 164 189 2 (set (reg:DI 2 cx [128])
        (const_int -4252017623040 [0xfffffc2200000000])) "csttest.c":25 85
{*movdi_internal}
     (expr_list:REG_EQUIV (const_int -4252017623040 [0xfffffc2200000000])
        (nil)))
(note 189 74 75 2 (var_location p (mem/c:DI (reg/f:DI 7 sp) [2 %sfp+-8 S8
A64])) NOTE_INSN_VAR_LOCATION)
... 
and we find the wrong location for p in note 189.

I've tracked this down to how and where insns are parsed into micro-ops.

The first insn:
...
(insn/f 132 4 133 2 (set (mem:DI (pre_dec:DI (reg/f:DI 7 sp)) [0  S8 A8])
        (reg:DI 44 r15)) "csttest.c":10 61 {*pushdi2_rex64}
     (expr_list:REG_DEAD (reg:DI 44 r15)
        (nil)))
...
is parsed into these micro-ops:
...
bb 2 op 0 insn 132 MO_ADJUST (set (mem:DI (pre_dec:DI (reg/f:DI 7 sp)) [0  S8
A8])
        (reg:DI 44 r15))
bb 2 op 1 insn 132 MO_VAL_USE (concat/v:DI (value/u:DI 8:8
@0x4c59428/0x4c89500)
        (reg:DI 44 r15))
bb 2 op 2 insn 132 MO_VAL_SET (concat/u:DI (value/u:DI 7:4306
@0x4c59410/0x4c894d0)
        (set (reg/f:DI 7 sp)
            (plus:DI (reg/f:DI 16 argp)
                (const_int -24 [0xffffffffffffffe8]))))
bb 2 op 3 insn 132 MO_VAL_SET (concat/u:DI (concat:DI (value/u:DI 8:8
@0x4c59428/0x4c89500)
            (mem:DI (value/u:DI 7:4306 @0x4c59410/0x4c894d0) [0  S8 A8]))
        (mem:DI (plus:DI (reg/f:DI 16 argp)
                (const_int -24 [0xffffffffffffffe8])) [0  S8 A8]))
...
and then rewritten into:
...
(insn/f 132 4 133 2 (parallel [
            (set (mem:DI (plus:DI (reg/f:DI 16 argp)
                        (const_int -24 [0xffffffffffffffe8])) [0  S8 A8])
                (reg:DI 44 r15))
            (set (reg/f:DI 7 sp)
                (plus:DI (reg/f:DI 16 argp)
                    (const_int -24 [0xffffffffffffffe8])))
        ]) "csttest.c":10 61 {*pushdi2_rex64}
     (expr_list:REG_DEAD (reg:DI 44 r15)
        (nil)))
...
This is incorrect, the offset should be -16 (8 for the pre_dec:DI, and 8 for
INCOMING_FRAME_SP_OFFSET), not -24.

AFAIU, the problem originates here in vt_initialize:
...
                  if (!frame_pointer_needed)
                    {
                      insn_stack_adjust_offset_pre_post (insn, &pre, &post);
                      if (pre)
                        {
                          micro_operation mo;
                          mo.type = MO_ADJUST;
                          mo.u.adjust = pre;
                          mo.insn = insn;
                          if (dump_file && (dump_flags & TDF_DETAILS))
                            log_op_type (PATTERN (insn), bb, insn,
                                         MO_ADJUST, dump_file);
                          VTI (bb)->mos.safe_push (mo);
                          VTI (bb)->out.stack_adjust += pre;
                        }
                    }

                  cselib_hook_called = false;
                  adjust_insn (bb, insn);
...

We start out with a 'VTI (bb)->out.stack_adjust' of 8 (equal to
INCOMING_FRAME_SP_OFFSET).

Then we process insn 132, and find a pre of 8 (due to the pre_dec:DI) and add
it, setting the stack_adjust to 16:
...
                          VTI (bb)->out.stack_adjust += pre;
...

Finally we call adjust_insn, where the stack_adjust is used in adjust_mems to
generate an sp equivalent, resulting in argp - 16:
...
      if (loc == stack_pointer_rtx
          && !frame_pointer_needed
          && cfa_base_rtx)
        return compute_cfa_pointer (amd->stack_adjust);
...

However, also pre_dec is handled in adjust_mems:
...
    case PRE_DEC:
      size = GET_MODE_SIZE (amd->mem_mode);
      addr = plus_constant (GET_MODE (loc), XEXP (loc, 0),
                            GET_CODE (loc) == PRE_INC ? size : -size);
      /* FALLTHRU */
...
so pre_dec is counted twice, once in the stack_adjust, and once as operator and
we end up with offset -24 instead of -16.


More information about the Gcc-bugs mailing list