Do not count debug instructions when counting number of uses to reorder frame objects.

[danielcdh]

Summary

The code generation should be independent of the debug info.

[majnemer]

The change looks fine but you will need to add a test case.

[majnemer]

test/DebugInfo/COFF/local-variables.ll

30–59

It would probably be a good idea to capture 52, 44 and 48 in FileCheck variables so that the test is less sensitive to changes.

[danielcdh]

add test and update existing test with variable.

[majnemer]

test/DebugInfo/COFF/local-variables.ll

30–60

You should use these variables down below

[danielcdh]

test/DebugInfo/COFF/local-variables.ll

30–60

The tests down below are separate tests (OBJ v.s. ASM), and these variables has no reuse in that test. So do you mean that I do not check these offset values at all in the OBJ tests? I can remove the check for numbers if that is what you meant.

[majnemer]

LGTM with the suggested changes to local-variables.ll undone.

test/DebugInfo/COFF/local-variables.ll

30–60

Ah, nevermind. In that case the test is better off with an explicit offset... Sorry about that!

[danielcdh]

undo changes in local-variables.ll

lib/Target/X86/X86FrameLowering.cpp

if (ObjectSize == 0)
       SortingObjects[Obj].ObjectSize = 4;
     else      
       SortingObjects[Obj].ObjectSize = ObjectSize;
   }
 
   // Count the number of uses for each object.
   for (auto &MBB : MF) {
     for (auto &MI : MBB) {
+      if (MI.isDebugValue())
+        continue;
       for (const MachineOperand &MO : MI.operands()) {
         // Check to see if it's a local stack symbol.
         if (!MO.isFI())
           continue;
         int Index = MO.getIndex();
         // Check to see if it falls within our range, and is tagged
         // to require ordering.
         if (Index >= 0 && Index < MFI->getObjectIndexEnd() &&

test/DebugInfo/COFF/local-variables.ll

 ; 17: }
 
 ; ASM: f:                                      # @f
 ; ASM:         .cv_file        1 "D:\\src\\llvm\\build\\t.cpp"
 ; ASM:         .cv_loc 0 1 7 0 is_stmt 0       # t.cpp:7:0
 ; ASM: .seh_proc f
 ; ASM: # BB#0:                                 # %entry
 ; ASM:         subq    $56, %rsp
-; ASM:         #DEBUG_VALUE: f:param <- [%RSP+44]
-; ASM:         movl    %ecx, 44(%rsp)
+; ASM:         #DEBUG_VALUE: f:param <- [%RSP+52]
+; ASM:         movl    %ecx, 52(%rsp)
 ; ASM: [[prologue_end:\.Ltmp.*]]:
 ; ASM:         .cv_loc 0 1 8 7                 # t.cpp:8:7
 ; ASM:         testl   %ecx, %ecx
 ; ASM:         je      .LBB0_2
 ; ASM: [[if_start:\.Ltmp.*]]:
 ; ASM: # BB#1:                                 # %if.then
-; ASM:         #DEBUG_VALUE: f:param <- [%RSP+44]
+; ASM:         #DEBUG_VALUE: f:param <- [%RSP+52]
 ; ASM:         #DEBUG_VALUE: a <- [%RSP+40]
 ; ASM:         .cv_loc 0 1 9 9                 # t.cpp:9:9
 ; ASM:         movl    $42, 40(%rsp)
 ; ASM: [[inline_site1:\.Ltmp.*]]:
 ; ASM:         .cv_loc 1 1 4 7                 # t.cpp:4:7
-; ASM:         movl    $3, 48(%rsp)
-; ASM:         leaq    48(%rsp), %rcx
+; ASM:         movl    $3, 44(%rsp)
+; ASM:         leaq    44(%rsp), %rcx
 ; ASM:         .cv_loc 1 1 5 3                 # t.cpp:5:3
 ; ASM:         callq   capture
 ; ASM:         leaq    40(%rsp), %rcx
 ; ASM:         jmp     .LBB0_3
 ; ASM: [[else_start:\.Ltmp.*]]:
 ; ASM: .LBB0_2:                                # %if.else
-; ASM:         #DEBUG_VALUE: f:param <- [%RSP+44]
+; ASM:         #DEBUG_VALUE: f:param <- [%RSP+52]
 ; ASM:         #DEBUG_VALUE: b <- [%RSP+36]
 ; ASM:         .cv_loc 0 1 13 9                # t.cpp:13:9
 ; ASM:         movl    $42, 36(%rsp)
 ; ASM: [[inline_site2:\.Ltmp.*]]:
 ; ASM:         .cv_loc 2 1 4 7                 # t.cpp:4:7
-; ASM:         movl    $3, 52(%rsp)
-; ASM:         leaq    52(%rsp), %rcx
+; ASM:         movl    $3, 48(%rsp)
+; ASM:         leaq    48(%rsp), %rcx
 ; ASM:         .cv_loc 2 1 5 3                 # t.cpp:5:3
 ; ASM:         callq   capture
 ; ASM:         leaq    36(%rsp), %rcx
 ; ASM: [[inline_site2_end:\.Ltmp.*]]:
 ; ASM: .LBB0_3:                                # %if.end
 ; ASM:         .cv_loc 0 1 15 5                # t.cpp:15:5
 ; ASM:         callq   capture
 ; ASM: [[else_end:\.Ltmp.*]]:
 ; ASM:         .cv_loc 0 1 17 1                # t.cpp:17:1
 ; ASM:         nop
 ; ASM:         addq    $56, %rsp
 ; ASM:         retq
 ; ASM: [[param_end:\.Ltmp.*]]:
 
 ; ASM: .short  4414                    # Record kind: S_LOCAL
 ; ASM: .long   116                     # TypeIndex
 ; ASM: .short  1                       # Flags
 ; ASM: .asciz  "param"
-; ASM: .cv_def_range    [[prologue_end]] [[param_end]], "E\021O\001\000\000,\000\000\000"
+; ASM: .cv_def_range    [[prologue_end]] [[param_end]], "E\021O\001\000\0004\000\000\000"
 ; ASM: .short  4414                    # Record kind: S_LOCAL
 ; ASM: .long   116                     # TypeIndex
 ; ASM: .short  0                       # Flags
 ; ASM: .asciz  "a"
 ; ASM: .cv_def_range    [[if_start]] [[else_start]], "E\021O\001\000\000(\000\000\000"
 ; ASM: .short  4414                    # Record kind: S_LOCAL
 ; ASM: .long   116                     # TypeIndex
 ; ASM: .short  0                       # Flags
 ; ASM: .asciz  "b"
 ; ASM: .cv_def_range    [[else_start]] [[else_end]], "E\021O\001\000\000$\000\000\000"
 ; ASM: .short  4429                    # Record kind: S_INLINESITE
 ; ASM: .short  4414                    # Record kind: S_LOCAL
 ; ASM: .long   116                     # TypeIndex
 ; ASM: .short  0                       # Flags
 ; ASM: .asciz  "v"
-; ASM: .cv_def_range    [[inline_site1]] [[else_start]], "E\021O\001\000\0000\000\000\000"
+; ASM: .cv_def_range    [[inline_site1]] [[else_start]], "E\021O\001\000\000,\000\000\000"
 ; ASM: .short  4430                    # Record kind: S_INLINESITE_END
 ; ASM: .short  4429                    # Record kind: S_INLINESITE
 ; ASM: .short  4414                    # Record kind: S_LOCAL
 ; ASM: .long   116                     # TypeIndex
 ; ASM: .short  0                       # Flags
 ; ASM: .asciz  "v"
-; ASM: .cv_def_range    [[inline_site2]] [[inline_site2_end]], "E\021O\001\000\0004\000\000\000"
+; ASM: .cv_def_range    [[inline_site2]] [[inline_site2_end]], "E\021O\001\000\0000\000\000\000"
 ; ASM: .short  4430                    # Record kind: S_INLINESITE_END
 
 ; OBJ:  Subsection [
 ; OBJ:    SubSectionType: Symbols (0xF1)
 ; OBJ:    ProcStart {
 ; OBJ:      DisplayName: f
 ; OBJ:      LinkageName: f
 ; OBJ:    }
 ; OBJ:    Local {
 ; OBJ:      Type: int (0x74)
 ; OBJ:      Flags [ (0x1)
 ; OBJ:        IsParameter (0x1)
 ; OBJ:      ]
 ; OBJ:      VarName: param
 ; OBJ:    }
 ; OBJ:    DefRangeRegisterRel {
 ; OBJ:      BaseRegister: 335
 ; OBJ:      HasSpilledUDTMember: No
 ; OBJ:      OffsetInParent: 0
-; OBJ:      BasePointerOffset: 44
+; OBJ:      BasePointerOffset: 52
 ; OBJ:      LocalVariableAddrRange {
 ; OBJ:        OffsetStart: .text+0x8
 ; OBJ:        ISectStart: 0x0
 ; OBJ:        Range: 0x4F
 ; OBJ:      }
 ; OBJ:    }
 ; OBJ:    Local {
 ; OBJ:      Type: int (0x74)

 ; OBJ:      Flags [ (0x0)
 ; OBJ:      ]
 ; OBJ:      VarName: v
 ; OBJ:    }
 ; OBJ:    DefRangeRegisterRel {
 ; OBJ:      BaseRegister: 335
 ; OBJ:      HasSpilledUDTMember: No
 ; OBJ:      OffsetInParent: 0
-; OBJ:      BasePointerOffset: 48
+; OBJ:      BasePointerOffset: 44
 ; OBJ:      LocalVariableAddrRange {
 ; OBJ:        OffsetStart: .text+0x14
 ; OBJ:        ISectStart: 0x0
 ; OBJ:        Range: 0x19
 ; OBJ:      }
 ; OBJ:    }
 ; OBJ:    InlineSiteEnd {
 ; OBJ:    }

 ; OBJ:      Flags [ (0x0)
 ; OBJ:      ]
 ; OBJ:      VarName: v
 ; OBJ:    }
 ; OBJ:    DefRangeRegisterRel {
 ; OBJ:      BaseRegister: 335
 ; OBJ:      HasSpilledUDTMember: No
 ; OBJ:      OffsetInParent: 0
-; OBJ:      BasePointerOffset: 52
+; OBJ:      BasePointerOffset: 48
 ; OBJ:      LocalVariableAddrRange {
 ; OBJ:        OffsetStart: .text+0x35
 ; OBJ:        ISectStart: 0x0
 ; OBJ:        Range: 0x17
 ; OBJ:      }
 ; OBJ:    }
 ; OBJ:    InlineSiteEnd {
 ; OBJ:    }