This is an archive of the discontinued LLVM Phabricator instance.

[DEBUG_INFO][NVPTX]Fix processing of DBG_VALUES.
ClosedPublic

Authored by ABataev on Oct 24 2018, 9:44 AM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

ABataev created this revision.Oct 24 2018, 9:44 AM
tra added inline comments.Oct 24 2018, 10:20 AM
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 added inline comments.Oct 24 2018, 10:33 AM
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 added inline comments.Oct 24 2018, 10:58 AM
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 updated this revision to Diff 170955.Oct 24 2018, 12:35 PM

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.

tra accepted this revision.Oct 24 2018, 2:04 PM
This revision is now accepted and ready to land.Oct 24 2018, 2:04 PM
This revision was automatically updated to reflect the committed changes.