[MIR] Add support for debug metadata for fixed stack objects

[thegameg]

Summary

Debug var, expr and loc were only supported for non-fixed stack objects.

This patch adds the following fields to the "fixedStack:" entries, and renames the ones from "stack:" to:

• debug-info-variable
• debug-info-expression
• debug-info-location

[aprantl]

include/llvm/CodeGen/MIRYamlMapping.h

329

I find that spelling weird: I would either go for the full debug-info-variable, or debug-variable, or dbg-variable, if it has to be lower-case.

[plotfi]

include/llvm/CodeGen/MachineFunction.h

352

Why the change in type here?

lib/CodeGen/MIRPrinter.cpp

277

Could this be cleaned up? Something like:

SmallVector<T, 3> DebugValues = { Object.DebugVar.Value, Object.DebugExpr.Value, Object.DebugLoc.Value };
SmallVector<T, 3> DebugVars = { DebugVar.Var, DebugVar.Expr, DebugVar.Loc };

for (unsigned i = 0; i < DebugValues.size(); ++i) {
    raw_string_ostream StrOS(DebugValues[i]);
    DebugVars[i]->printAsOperand(StrOS, MST);
}

[thegameg]

• Rename di-* to debug-info-* for fixedStack: and stack:
• Add comments to test to explain what they're testing

[thegameg]

include/llvm/CodeGen/MachineFunction.h

352

Fixed stack objects have negative FrameIndex values.

lib/CodeGen/MIRPrinter.cpp

277

I'm afraid this is not going to work since DebugVar.Var, .Expr and .Loc have different types.

[thegameg]

Fix a few tests.

[JDevlieghere]

include/llvm/CodeGen/MachineFunction.h

352

Might be worth adding a comment saying exactly that?

test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll

36 ↗

(On Diff #143892)

In the MappingTraits you say that it shouldn't print if it's empty, which makes sense, but then it's still being printed here?

[thegameg]

Add comment for int Slot;.

test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll

36 ↗

(On Diff #143892)

yaml::Output has a WriteDefaultValues flag which ignores default values when printing them. Passing -simplify-mir to llc will skip printing default values.

[JDevlieghere]

In D46032#1077921, @thegameg wrote:

Add comment for int Slot;.

Alright, fair enough. Thanks Francis, this LGTM.

include/llvm/CodeGen/MIRYamlMapping.h

struct FixedMachineStackObject {
   int64_t Offset = 0;
   uint64_t Size = 0;
   unsigned Alignment = 0;
   uint8_t StackID = 0;
   bool IsImmutable = false;
   bool IsAliased = false;
   StringValue CalleeSavedRegister;
   bool CalleeSavedRestored = true;
+  StringValue DebugVar;
+  StringValue DebugExpr;
+  StringValue DebugLoc;
 
   bool operator==(const FixedMachineStackObject &Other) const {
     return ID == Other.ID && Type == Other.Type && Offset == Other.Offset &&
            Size == Other.Size && Alignment == Other.Alignment &&
            StackID == Other.StackID &&
            IsImmutable == Other.IsImmutable && IsAliased == Other.IsAliased &&
            CalleeSavedRegister == Other.CalleeSavedRegister &&
-           CalleeSavedRestored == Other.CalleeSavedRestored;
+           CalleeSavedRestored == Other.CalleeSavedRestored &&
+           DebugVar == Other.DebugVar && DebugExpr == Other.DebugExpr
+           && DebugLoc == Other.DebugLoc;
   }
 };
 
 template <>
 struct ScalarEnumerationTraits<FixedMachineStackObject::ObjectType> {
   static void enumeration(yaml::IO &IO,
                           FixedMachineStackObject::ObjectType &Type) {
     IO.enumCase(Type, "default", FixedMachineStackObject::DefaultType);
static void mapping(yaml::IO &YamlIO, FixedMachineStackObject &Object) {
     if (Object.Type != FixedMachineStackObject::SpillSlot) {
       YamlIO.mapOptional("isImmutable", Object.IsImmutable, false);
       YamlIO.mapOptional("isAliased", Object.IsAliased, false);
     }
     YamlIO.mapOptional("callee-saved-register", Object.CalleeSavedRegister,
                        StringValue()); // Don't print it out when it's empty.
     YamlIO.mapOptional("callee-saved-restored", Object.CalleeSavedRestored,
                      true);
+    YamlIO.mapOptional("di-variable", Object.DebugVar,
+                       StringValue()); // Don't print it out when it's empty.
+    YamlIO.mapOptional("di-expression", Object.DebugExpr,
+                       StringValue()); // Don't print it out when it's empty.
+    YamlIO.mapOptional("di-location", Object.DebugLoc,
+                       StringValue()); // Don't print it out when it's empty.
   }
 
   static const bool flow = true;
 };
 
 struct MachineConstantPoolValue {
   UnsignedValue ID;
   StringValue Value;

include/llvm/CodeGen/MachineFunction.h

class MachineFunction {
   /// In particular, the XXXInfo data structure.
   /// \pre Fn, Target, MMI, and FunctionNumber are properly set.
   void init();
 
 public:
   struct VariableDbgInfo {
     const DILocalVariable *Var;
     const DIExpression *Expr;
-    unsigned Slot;
+    int Slot;
     const DILocation *Loc;
 
     VariableDbgInfo(const DILocalVariable *Var, const DIExpression *Expr,
-                    unsigned Slot, const DILocation *Loc)
+                    int Slot, const DILocation *Loc)
         : Var(Var), Expr(Expr), Slot(Slot), Loc(Loc) {}
   };
   using VariableDbgInfoMapTy = SmallVector<VariableDbgInfo, 4>;
   VariableDbgInfoMapTy VariableDbgInfos;
 
   MachineFunction(const Function &F, const TargetMachine &TM,
                   const TargetSubtargetInfo &STI, unsigned FunctionNum,
                   MachineModuleInfo &MMI);
public:
   const std::vector<unsigned> &getFilterIds() const {
     return FilterIds;
   }
 
   /// \}
 
   /// Collect information used to emit debugging information of a variable.
   void setVariableDbgInfo(const DILocalVariable *Var, const DIExpression *Expr,
-                          unsigned Slot, const DILocation *Loc) {
+                          int Slot, const DILocation *Loc) {
     VariableDbgInfos.emplace_back(Var, Expr, Slot, Loc);
   }
 
   VariableDbgInfoMapTy &getVariableDbgInfo() { return VariableDbgInfos; }
   const VariableDbgInfoMapTy &getVariableDbgInfo() const {
     return VariableDbgInfos;
   }
 };

lib/CodeGen/MIRParser/MIRParser.cpp

public:
   bool initializeFrameInfo(PerFunctionMIParsingState &PFS,
                            const yaml::MachineFunction &YamlMF);
 
   bool parseCalleeSavedRegister(PerFunctionMIParsingState &PFS,
                                 std::vector<CalleeSavedInfo> &CSIInfo,
                                 const yaml::StringValue &RegisterSource,
                                 bool IsRestored, int FrameIdx);
 
+  template <typename T>
   bool parseStackObjectsDebugInfo(PerFunctionMIParsingState &PFS,
-                                  const yaml::MachineStackObject &Object,
+                                  const T &Object,
                                   int FrameIdx);
 
   bool initializeConstantPool(PerFunctionMIParsingState &PFS,
                               MachineConstantPool &ConstantPool,
                               const yaml::MachineFunction &YamlMF);
 
   bool initializeJumpTableInfo(PerFunctionMIParsingState &PFS,
                                const yaml::MachineJumpTable &YamlJTI);
if (!PFS.FixedStackObjectSlots.insert(std::make_pair(Object.ID.Value,
                                                          ObjectIdx))
              .second)
       return error(Object.ID.SourceRange.Start,
                    Twine("redefinition of fixed stack object '%fixed-stack.") +
                        Twine(Object.ID.Value) + "'");
     if (parseCalleeSavedRegister(PFS, CSIInfo, Object.CalleeSavedRegister,
                                  Object.CalleeSavedRestored, ObjectIdx))
       return true;
+    if (parseStackObjectsDebugInfo(PFS, Object, ObjectIdx))
+      return true;
   }
 
   // Initialize the ordinary frame objects.
   for (const auto &Object : YamlMF.StackObjects) {
     int ObjectIdx;
     const AllocaInst *Alloca = nullptr;
     const yaml::StringValue &Name = Object.Name;
     if (!Name.Value.empty()) {
static bool typecheckMDNode(T *&Result, MDNode *Node,
   Result = dyn_cast<T>(Node);
   if (!Result)
     return Parser.error(Source.SourceRange.Start,
                         "expected a reference to a '" + TypeString +
                             "' metadata node");
   return false;
 }
 
+template <typename T>
 bool MIRParserImpl::parseStackObjectsDebugInfo(PerFunctionMIParsingState &PFS,
-    const yaml::MachineStackObject &Object, int FrameIdx) {
+    const T &Object, int FrameIdx) {
   // Debug information can only be attached to stack objects; Fixed stack
   // objects aren't supported.
-  assert(FrameIdx >= 0 && "Expected a stack object frame index");
   MDNode *Var = nullptr, *Expr = nullptr, *Loc = nullptr;
   if (parseMDNode(PFS, Var, Object.DebugVar) ||
       parseMDNode(PFS, Expr, Object.DebugExpr) ||
       parseMDNode(PFS, Loc, Object.DebugLoc))
     return true;
   if (!Var && !Expr && !Loc)
     return false;
   DILocalVariable *DIVar = nullptr;
   DIExpression *DIExpr = nullptr;
   DILocation *DILoc = nullptr;
   if (typecheckMDNode(DIVar, Var, Object.DebugVar, "DILocalVariable", *this) ||
       typecheckMDNode(DIExpr, Expr, Object.DebugExpr, "DIExpression", *this) ||
       typecheckMDNode(DILoc, Loc, Object.DebugLoc, "DILocation", *this))
     return true;
-  PFS.MF.setVariableDbgInfo(DIVar, DIExpr, unsigned(FrameIdx), DILoc);
+  PFS.MF.setVariableDbgInfo(DIVar, DIExpr, FrameIdx, DILoc);
   return false;
 }
 
 bool MIRParserImpl::parseMDNode(PerFunctionMIParsingState &PFS,
     MDNode *&Node, const yaml::StringValue &Source) {
   if (Source.Value.empty())
     return false;
   SMDiagnostic Error;

lib/CodeGen/MIRPrinter.cpp

 
 static void printRegClassOrBank(unsigned Reg, yaml::StringValue &Dest,
                                 const MachineRegisterInfo &RegInfo,
                                 const TargetRegisterInfo *TRI) {
   raw_string_ostream OS(Dest.Value);
   OS << printRegClassOrBank(Reg, RegInfo, TRI);
 }
 
+template <typename T>
+static void
+printStackObjectDbgInfo(const MachineFunction::VariableDbgInfo &DebugVar,
+                        T &Object, ModuleSlotTracker &MST) {
+  {
+    raw_string_ostream StrOS(Object.DebugVar.Value);
+    DebugVar.Var->printAsOperand(StrOS, MST);
+  }
+
+  {
+    raw_string_ostream StrOS(Object.DebugExpr.Value);
+    DebugVar.Expr->printAsOperand(StrOS, MST);
+  }
+
+  {
+    raw_string_ostream StrOS(Object.DebugLoc.Value);
+    DebugVar.Loc->printAsOperand(StrOS, MST);
+  }
+}
 
 void MIRPrinter::convert(yaml::MachineFunction &MF,
                          const MachineRegisterInfo &RegInfo,
                          const TargetRegisterInfo *TRI) {
   MF.TracksRegLiveness = RegInfo.tracksLiveness();
 
   // Print the virtual register definitions.
   for (unsigned I = 0, E = RegInfo.getNumVirtRegs(); I < E; ++I) {
void MIRPrinter::convertStackObjects(yaml::MachineFunction &YMF,
 
   // Print the debug variable information.
   for (const MachineFunction::VariableDbgInfo &DebugVar :
        MF.getVariableDbgInfo()) {
     auto StackObjectInfo = StackObjectOperandMapping.find(DebugVar.Slot);
     assert(StackObjectInfo != StackObjectOperandMapping.end() &&
            "Invalid stack object index");
     const FrameIndexOperand &StackObject = StackObjectInfo->second;
-    assert(!StackObject.IsFixed && "Expected a non-fixed stack object");
+    if (StackObject.IsFixed) {
+      auto &Object = YMF.FixedStackObjects[StackObject.ID];
+      printStackObjectDbgInfo(DebugVar, Object, MST);
+    } else {
       auto &Object = YMF.StackObjects[StackObject.ID];
-    {
-      raw_string_ostream StrOS(Object.DebugVar.Value);
-      DebugVar.Var->printAsOperand(StrOS, MST);
-    }
-    {
-      raw_string_ostream StrOS(Object.DebugExpr.Value);
-      DebugVar.Expr->printAsOperand(StrOS, MST);
-    }
-    {
-      raw_string_ostream StrOS(Object.DebugLoc.Value);
-      DebugVar.Loc->printAsOperand(StrOS, MST);
+      printStackObjectDbgInfo(DebugVar, Object, MST);
     }
   }
 }
 
 void MIRPrinter::convert(yaml::MachineFunction &MF,
                          const MachineConstantPool &ConstantPool) {
   unsigned ID = 0;
   for (const MachineConstantPoolEntry &Constant : ConstantPool.getConstants()) {

test/CodeGen/MIR/X86/callee-saved-info.mir

 name:            func
 tracksRegLiveness: true
 frameInfo:
   stackSize:       24
   maxAlignment:    4
   adjustsStack:    true
   hasCalls:        true
 # CHECK: fixedStack:
-# CHECK: callee-saved-register: '$rbx', callee-saved-restored: true }
+# CHECK: callee-saved-register: '$rbx', callee-saved-restored: true
 fixedStack:
   - { id: 0, type: spill-slot, offset: -16, size: 8, alignment: 16, callee-saved-register: '$rbx' }
 # CHECK: stack:
 # CHECK-NEXT: - { id: 0
 # CHECK: callee-saved-register: '$edi', callee-saved-restored: false
 stack:
   - { id: 0, name: b, offset: -20, size: 4, alignment: 4 }
   - { id: 1, offset: -24, size: 4, alignment: 4, callee-saved-register: '$edi',

test/CodeGen/MIR/X86/fixed-stack-di.mir

+# RUN: llc -mtriple=x86_64-apple-unknown -run-pass none %s -o /dev/null
+
+--- |
+
+  target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-apple-unknown"
+
+  declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+
+  define hidden void @foo(i32* byval %dstRect) {
+  entry:
+    call void @llvm.dbg.declare(metadata i32* %dstRect, metadata !3, metadata !DIExpression()), !dbg !5
+    unreachable
+  }
+
+  attributes #0 = { nounwind readnone speculatable }
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!2}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1)
+  !1 = !DIFile(filename: "file.cpp", directory: "/dir")
+  !2 = !{i32 2, !"Debug Info Version", i32 3}
+  !3 = !DILocalVariable(name: "dstRect", scope: !4)
+  !4 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !0, file: !1, line: 42, unit: !0)
+  !5 = !DILocation(line: 42, column: 85, scope: !4)
+
+...
+---
+name:            foo
+alignment:       4
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    8
+fixedStack:
+  - { id: 0, size: 4, alignment: 16, stack-id: 0, di-variable: '!3', di-expression: '!DIExpression()',
+      di-location: '!5' }
+body:             |
+  bb.0.entry:
+
+...

test/CodeGen/MIR/X86/spill-slot-fixed-stack-objects.mir

 
 ...
 ---
 name:            test
 frameInfo:
   maxAlignment:    4
 # CHECK: fixedStack:
 # CHECK-NEXT: - { id: 0, type: spill-slot, offset: 0, size: 4, alignment: 4, stack-id: 0,
-# CHECK-NEXT: callee-saved-register: '', callee-saved-restored: true }
+# CHECK-NEXT: callee-saved-register: '', callee-saved-restored: true, di-variable: '',
+# CHECK-NEXT: di-expression: '', di-location: '' }
 fixedStack:
   - { id: 0, type: spill-slot, offset: 0, size: 4, alignment: 4 }
 stack:
   - { id: 0, offset: -12, size: 4, alignment: 4 }
 body: |
   bb.0.entry:
     MOV32mr $rsp, 1, _, -4, _, $edi
     $eax = COPY $edi
     RETQ $eax
 ...

test/CodeGen/X86/fixed-stack-di-mir.ll

+; RUN: llc -mtriple=x86_64-apple-unknown -stop-before=expand-isel-pseudos %s -o - -simplify-mir | FileCheck %s
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-unknown"
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+
+define hidden void @foo(i32* byval %dstRect) {
+; CHECK-LABEL: name: foo
+entry:
+  call void @llvm.dbg.declare(metadata i32* %dstRect, metadata !3, metadata !DIExpression()), !dbg !5
+; CHECK: fixedStack:
+; CHECK: id: 0
+; CHECK: di-variable: '!3'
+; CHECK: di-expression: '!DIExpression()'
+; CHECK: di-location: '!5'
+  unreachable
+}
+
+attributes #0 = { nounwind readnone speculatable }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1)
+!1 = !DIFile(filename: "file.cpp", directory: "/dir")
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!3 = !DILocalVariable(name: "dstRect", scope: !4)
+!4 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !0, file: !1, line: 42, unit: !0)
+!5 = !DILocation(line: 42, column: 85, scope: !4)