X86FrameLowering: Fix getFrameIndexReference() for fixed objects

[MatzeB]

Summary

Debug information is calculated with getFrameIndexReference() which was
missing some logic for the fixed object cases (= parameters on the stack).

The respective logic was already present in X86RegisterInfo::eliminateFrameIndex(). I'll submit separate patches to avoid the code duplication!

[aprantl]

Thanks! This LGTM from the debug info side of things. (One comment inline)

test/CodeGen/X86/dbg-baseptr.ll

4 ↗

(On Diff #95661)

Could you add the source code this was generated from as a comment? This helps with future upgrades.

[MatzeB]

test/CodeGen/X86/dbg-baseptr.ll

4 ↗

(On Diff #95661)

There's no sourcecode: I manually crafted this (unfortunately it turned out more complicated than I hoped with the debug info around)

[aprantl]

okay — works for me.

llvm/trunk/lib/Target/X86/X86FrameLowering.cpp

if (Offset) {
 
       // Check for possible merge with preceding ADD instruction.
       Offset += mergeSPUpdates(MBB, MBBI, true);
       emitSPUpdate(MBB, MBBI, Offset, /*InEpilogue=*/true);
     }
   }
 }
 
-// NOTE: this only has a subset of the full frame index logic. In
-// particular, the FI < 0 and AfterFPPop logic is handled in
-// X86RegisterInfo::eliminateFrameIndex, but not here. Possibly
-// (probably?) it should be moved into here.
 int X86FrameLowering::getFrameIndexReference(const MachineFunction &MF, int FI,
                                              unsigned &FrameReg) const {
   const MachineFrameInfo &MFI = MF.getFrameInfo();
 
+  bool IsFixed = MFI.isFixedObjectIndex(FI);
   // We can't calculate offset from frame pointer if the stack is realigned,
   // so enforce usage of stack/base pointer.  The base pointer is used when we
   // have dynamic allocas in addition to dynamic realignment.
   if (TRI->hasBasePointer(MF))
-    FrameReg = TRI->getBaseRegister();
+    FrameReg = IsFixed ? TRI->getFramePtr() : TRI->getBaseRegister();
   else if (TRI->needsStackRealignment(MF))
-    FrameReg = TRI->getStackRegister();
+    FrameReg = IsFixed ? TRI->getFramePtr() : TRI->getStackRegister();
   else
     FrameReg = TRI->getFrameRegister(MF);
 
   // Offset will hold the offset from the stack pointer at function entry to the
   // object.
   // We need to factor in additional offsets applied during the prologue to the
   // frame, base, and stack pointer depending on which is used.
   int Offset = MFI.getObjectOffset(FI) - getOffsetOfLocalArea();

llvm/trunk/lib/Target/X86/X86RegisterInfo.h

void eliminateFrameIndex(MachineBasicBlock::iterator MI,
                            int SPAdj, unsigned FIOperandNum,
                            RegScavenger *RS = nullptr) const override;
 
   // Debug information queries.
   unsigned getFrameRegister(const MachineFunction &MF) const override;
   unsigned getPtrSizedFrameRegister(const MachineFunction &MF) const;
   unsigned getStackRegister() const { return StackPtr; }
   unsigned getBaseRegister() const { return BasePtr; }
+  /// Returns physical register used as frame pointer.
+  /// This will always returns the frame pointer register, contrary to
+  /// getFrameRegister() which returns the "base pointer" in situations
+  /// involving a stack, frame and base pointer.
+  unsigned getFramePtr() const { return FramePtr; }
   // FIXME: Move to FrameInfok
   unsigned getSlotSize() const { return SlotSize; }
 };
 
 } // End llvm namespace
 
 #endif

llvm/trunk/test/CodeGen/X86/dbg-baseptr.ll

+; RUN: llc -o - %s | FileCheck %s
+; This test checks that parameters on the stack pointer are correctly
+; referenced by debug info.
+target triple = "x86_64--"
+
+@glob = external global i64
+@ptr = external global i32*
+%struct.s = type { i32, i32, i32, i32, i32 }
+
+; CHECK-LABEL: f0:
+; CHECK: DEBUG_VALUE: f:input <- [%RSP+8]
+define i32 @f0(%struct.s* byval align 8 %input) !dbg !8 {
+  call void @llvm.dbg.declare(metadata %struct.s* %input, metadata !4, metadata !17), !dbg !18
+  ret i32 42
+}
+
+; CHECK-LABEL: f1:
+; CHECK: DEBUG_VALUE: f:input <- [%RBP+16]
+define i32 @f1(%struct.s* byval align 8 %input) !dbg !8 {
+  %val = load i64, i64* @glob
+  ; this alloca should force FP usage.
+  %stackspace = alloca i32, i64 %val, align 1
+  store i32* %stackspace, i32** @ptr
+  call void @llvm.dbg.declare(metadata %struct.s* %input, metadata !4, metadata !17), !dbg !18
+  ret i32 42
+}
+
+; CHECK-LABEL: f2:
+; Just check that we are indeed aligning the stack and setting up a base pointer
+; in RBX.
+; CHECK: pushq %rbp
+; CHECK: movq %rsp, %rbp
+; CHECK: pushq %rbx
+; CHECK: andq $-64, %rsp
+; CHECK: subq $64, %rsp
+; CHECK: movq %rsp, %rbx
+; The parameter should still be referenced through RBP though.
+; CHECK-NOT: DEBUG_VALUE: f:input <- [%RBX
+; CHECK: DEBUG_VALUE: f:input <- [%RBP+16]
+define i32 @f2(%struct.s* byval align 8 %input) !dbg !8 {
+  %val = load i64, i64* @glob
+  %stackspace = alloca i32, i64 %val, align 64
+  store i32* %stackspace, i32** @ptr
+  call void @llvm.dbg.declare(metadata %struct.s* %input, metadata !4, metadata !17), !dbg !18
+  ret i32 42
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!0, !1}
+
+!0 = !{i32 2, !"Dwarf Version", i32 4}
+!1 = !{i32 2, !"Debug Info Version", i32 3}
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3)
+!3 = !DIFile(filename: "dbg-baseptr.ll", directory: "/")
+!4 = !DILocalVariable(name: "input", arg: 1, scope: !8, file: !3, line: 5, type: !9)
+!5 = !{}
+
+!6 = !DISubroutineType(types: !7)
+!7 = !{!10, !9}
+
+!8 = distinct !DISubprogram(name: "f", file: !3, line: 5, type: !6, isLocal: false, isDefinition: true, flags: DIFlagPrototyped, unit: !2, variables: !5)
+
+!9 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "s", elements: !11)
+!10 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
+!11 = !{!12, !13, !14, !15, !16}
+!12 = !DIDerivedType(tag: DW_TAG_member, name: "a", baseType: !10, size: 32)
+!13 = !DIDerivedType(tag: DW_TAG_member, name: "b", baseType: !10, size: 32, offset: 32)
+!14 = !DIDerivedType(tag: DW_TAG_member, name: "c", baseType: !10, size: 32, offset: 64)
+!15 = !DIDerivedType(tag: DW_TAG_member, name: "d", baseType: !10, size: 32, offset: 96)
+!16 = !DIDerivedType(tag: DW_TAG_member, name: "e", baseType: !10, size: 32, offset: 128)
+
+!17 = !DIExpression()
+!18 = !DILocation(line: 5, scope: !8)