[DEBUG_INFO][NVPTX]Fix processing of DBG_VALUES.

[ABataev]

Summary

If the instruction in the eliminateFrameIndex function is a DBG_VALUE
instruction, it requires special processing. The frame register is set
to VRFrame and the offset is set basing on object offset.

[tra]

lib/Target/NVPTX/NVPTXRegisterInfo.cpp

122–129 ↗

(On Diff #170923)

I'm not sure I understand this change. What is in getOperand(FIOperandNum + 1) for DebugValue instructions? Other instructions carry FI-based offset, but your change appears to ignore it and effectively treats it as if it's always zero.

It may be worth adding more details about *why* debug values need to be handled this way.

126 ↗

(On Diff #170923)

nit: currently the code below it appears to do special processing of *non*-DebugValue instructions.

[ABataev]

lib/Target/NVPTX/NVPTXRegisterInfo.cpp

122–129 ↗

(On Diff #170923)

DebugValue does not have FI-based offset. That's why it causes compiler crash. For DebugValue it is enough just to have the base object offset.
Some other targets, like ARC, Thumb have same kind of processing.
Alternatively, we can use the code similar to one existing in lib/CodeGen/PrologEpilogInserter.cpp for special handling of the debug values:

// Frame indices in debug values are encoded in a target independent
// way with simply the frame index and offset rather than any
// target-specific addressing mode.
if (MI.isDebugValue()) {
  assert(i == 0 && "Frame indices can only appear as the first "
                   "operand of a DBG_VALUE machine instruction");
  unsigned Reg;
  int64_t Offset =
      TFI->getFrameIndexReference(MF, MI.getOperand(0).getIndex(), Reg);
  MI.getOperand(0).ChangeToRegister(Reg, false /*isDef*/);
  MI.getOperand(0).setIsDebug();
  auto *DIExpr = DIExpression::prepend(MI.getDebugExpression(),
                                       DIExpression::NoDeref, Offset);
  MI.getOperand(3).setMetadata(DIExpr);
  continue;
}

[tra]

lib/Target/NVPTX/NVPTXRegisterInfo.cpp

122–129 ↗

(On Diff #170923)

OK. Adding the comment that DebugValue uses different encoding convention that does not need in-object offset should do.

Perhaps the intention could be made a bit more obvious if the code was structured like this:

// Explain why DebugValue has InObjectOffset = 0.
InObjectOffset = MI.isDebugValue() ? 0 : MI.getOperand(FIOperandNum + 1).getImm()
Offset = MF.getFrameInfo().getObjectOffset(FrameIndex) + InObjectOffset;

[ABataev]

Reworked processing of the DBG_Value. Used the code from
lib/CodeGen/PrologEpilogInserter.cpp instead of the custom processing. Seems to me,
this implementation is more robust than the original one.

lib/Target/NVPTX/NVPTXPrologEpilogPass.cpp

 
 #include "NVPTX.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/TargetFrameLowering.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
+#include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "nvptx-prolog-epilog"
 
bool NVPTXPrologEpilogPass::runOnMachineFunction(MachineFunction &MF) {
 
   calculateFrameObjectOffsets(MF);
 
   for (MachineBasicBlock &MBB : MF) {
     for (MachineInstr &MI : MBB) {
       for (unsigned i = 0, e = MI.getNumOperands(); i != e; ++i) {
         if (!MI.getOperand(i).isFI())
           continue;
+
+        // Frame indices in debug values are encoded in a target independent
+        // way with simply the frame index and offset rather than any
+        // target-specific addressing mode.
+        if (MI.isDebugValue()) {
+          assert(i == 0 && "Frame indices can only appear as the first "
+                           "operand of a DBG_VALUE machine instruction");
+          unsigned Reg;
+          int64_t Offset =
+              TFI.getFrameIndexReference(MF, MI.getOperand(0).getIndex(), Reg);
+          MI.getOperand(0).ChangeToRegister(Reg, /*isDef=*/false);
+          MI.getOperand(0).setIsDebug();
+          auto *DIExpr = DIExpression::prepend(MI.getDebugExpression(),
+                                               DIExpression::NoDeref, Offset);
+          MI.getOperand(3).setMetadata(DIExpr);
+          continue;
+        }
+
         TRI.eliminateFrameIndex(MI, 0, i, nullptr);
         Modified = true;
       }
     }
   }
 
   // Add function prolog/epilog
   TFI.emitPrologue(MF, MF.front());

test/DebugInfo/NVPTX/dbg-value-const-byref.ll

+; RUN: llc -mtriple=nvptx64-nvidia-cuda < %s | FileCheck %s
+; Generated with -O1 from:
+; int f1();
+; void f2(int*);
+; int f3(int);
+;
+; int foo() {
+;   int i = 3;
+;   f3(i);
+;   i = 7;
+;   i = f1();
+;   f2(&i);
+;   return 0;
+; }
+;
+; Test that we generate valid debug info for optimized code,
+; particularly variables that are described as constants and passed
+; by reference.
+;
+; CHECK: DEBUG_VALUE: foo:i <- [DW_OP_deref] $vrdepot
+; CHECK: DEBUG_VALUE: foo:i <- 3
+; CHECK: DEBUG_VALUE: foo:i <- 7
+; CHECK: DEBUG_VALUE: foo:i <- %
+
+; Function Attrs: nounwind ssp uwtable
+define i32 @foo() #0 !dbg !4 {
+entry:
+  %i = alloca i32, align 4
+  call void @llvm.dbg.value(metadata i32 3, metadata !10, metadata !DIExpression()), !dbg !15
+  %call = call i32 @f3(i32 3) #3, !dbg !16
+  call void @llvm.dbg.value(metadata i32 7, metadata !10, metadata !DIExpression()), !dbg !18
+  %call1 = call i32 (...) @f1() #3, !dbg !19
+  call void @llvm.dbg.value(metadata i32 %call1, metadata !10, metadata !DIExpression()), !dbg !19
+  store i32 %call1, i32* %i, align 4, !dbg !19, !tbaa !20
+  call void @llvm.dbg.value(metadata i32* %i, metadata !10, metadata !DIExpression(DW_OP_deref)), !dbg !24
+  call void @f2(i32* %i) #3, !dbg !24
+  ret i32 0, !dbg !25
+}
+
+declare i32 @f3(i32)
+
+declare i32 @f1(...)
+
+declare void @f2(i32*)
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.value(metadata, metadata, metadata) #2
+
+attributes #0 = { nounwind ssp uwtable }
+attributes #2 = { nounwind readnone }
+attributes #3 = { nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!11, !12}
+!llvm.ident = !{!13}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, producer: "clang version 3.5.0 ", isOptimized: true, emissionKind: FullDebug, file: !1, enums: !2, retainedTypes: !2, globals: !2, imports: !2)
+!1 = !DIFile(filename: "dbg-value-const-byref.c", directory: "")
+!2 = !{}
+!4 = distinct !DISubprogram(name: "foo", line: 5, isLocal: false, isDefinition: true, virtualIndex: 6, isOptimized: true, unit: !0, scopeLine: 5, file: !1, scope: !5, type: !6, retainedNodes: !9)
+!5 = !DIFile(filename: "dbg-value-const-byref.c", directory: "")
+!6 = !DISubroutineType(types: !7)
+!7 = !{!8}
+!8 = !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!9 = !{!10}
+!10 = !DILocalVariable(name: "i", line: 6, scope: !4, file: !5, type: !8)
+!11 = !{i32 2, !"Dwarf Version", i32 2}
+!12 = !{i32 1, !"Debug Info Version", i32 3}
+!13 = !{!"clang version 3.5.0 "}
+!14 = !{i32 3}
+!15 = !DILocation(line: 6, scope: !4)
+!16 = !DILocation(line: 7, scope: !4)
+!17 = !{i32 7}
+!18 = !DILocation(line: 8, scope: !4)
+!19 = !DILocation(line: 9, scope: !4)
+!20 = !{!21, !21, i64 0}
+!21 = !{!"int", !22, i64 0}
+!22 = !{!"omnipotent char", !23, i64 0}
+!23 = !{!"Simple C/C++ TBAA"}
+!24 = !DILocation(line: 10, scope: !4)
+!25 = !DILocation(line: 11, scope: !4)