This is an archive of the discontinued LLVM Phabricator instance.

[DebugInfo] Examine all uses of isDebugValue() for debug instructions.
ClosedPublic

Authored by HsiangKai on Apr 5 2018, 5:13 PM.

Details

Summary

Because we create a new kind of debug instruction, DBG_LABEL, we need to
check all passes which use isDebugValue() to check MachineInstr is debug
instruction or not. When expelling debug instructions, we should expel
both DBG_VALUE and DBG_LABEL. So, I create a new function,
isDebugInstr(), in MachineInstr to check whether the MachineInstr is
debug instruction or not.

This patch has no new test case. I have run regression test and there is
no difference in regression test.

Diff Detail

Repository
rL LLVM

Event Timeline

HsiangKai created this revision.Apr 5 2018, 5:13 PM
aprantl added inline comments.Apr 6 2018, 3:35 PM
include/llvm/CodeGen/MachineBasicBlock.h
893

It might be interesting so look at the uses and see if this shouldn't instead test for isMetaInstruction.

lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp
239

indentation

lib/CodeGen/ScheduleDAGInstrs.cpp
768

LLVM usually doesn't use braces around a single statement.

HsiangKai updated this revision to Diff 141999.Apr 11 2018, 7:03 AM

Update according to review comments.

aprantl accepted this revision.Apr 11 2018, 10:04 AM

You could land a simplified implementation of isDebugInstr that only checks for DBG_VALUE first and get this in before the other dependencies.

This revision is now accepted and ready to land.Apr 11 2018, 10:04 AM
Ka-Ka added a subscriber: Ka-Ka.Apr 11 2018, 11:08 PM
This revision was automatically updated to reflect the committed changes.
bjope added a subscriber: bjope.May 9 2018, 1:12 PM
bjope added inline comments.
llvm/trunk/lib/CodeGen/InlineSpiller.cpp
935 ↗(On Diff #145850)

Isn't this is a little bit confusing?
Calling buildDbgValueForSpill is probably only correct for a DBG_VALUE. So the old check for MI->isDebugValue() is probably more correct (and more future proof).

And then perhaps there should be an assertion like this on line 943 instead:

assert(!MI->isDebugInstr() && "Did not expect to find a use in debug instruction that isn't a DBG_VALUE")
HsiangKai marked an inline comment as done.May 9 2018, 8:08 PM
HsiangKai added inline comments.
llvm/trunk/lib/CodeGen/InlineSpiller.cpp
935 ↗(On Diff #145850)

I replaced isDebugValue() by isDebugInstr() conservatively. The instructions using Reg should not be DBG_LABEL. I think your opinion is right. I will create a patch to correct it.

HsiangKai marked 2 inline comments as done.May 10 2018, 8:09 PM

Create a new revision to review it.
https://reviews.llvm.org/D46739