Page MenuHomePhabricator

[Debugify] Strip added metadata in the -debugify-each pipeline
ClosedPublic

Authored by vsk on Apr 10 2020, 3:46 PM.

Details

Summary

Share logic to strip debugify metadata between the IR and MIR level
debugify passes. This makes it simpler to hunt for bugs by diffing IR
with vs. without -debugify-each turned on.

As a drive-by, fix an issue causing CallGraphNodes to become invalid
when a dead llvm.dbg.value prototype is deleted.

Diff Detail

Event Timeline

vsk created this revision.Apr 10 2020, 3:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 10 2020, 3:46 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
dsanders accepted this revision.Apr 10 2020, 4:28 PM

LGTM with the behavioural change to StripDebugMachineModule fixed

llvm/include/llvm/IR/IntrinsicInst.h
84–85

I think there might be potential for confusion between isa<DbgInfoIntrinsic>() and DbgInfoIntrinsic::isA(). Maybe isInstForInstrinsic(), or implementsInstrinsic()?

llvm/lib/Transforms/Utils/Debugify.cpp
183–185

This will stop StripDebugMachineModule stripping debug info when OnlyDebugified is false.

This revision is now accepted and ready to land.Apr 10 2020, 4:28 PM
vsk added a comment.Apr 10 2020, 5:05 PM

Well, it looks like arcanist ate my patch and spit out a different one. Let me try that again...

vsk updated this revision to Diff 256708.Apr 10 2020, 5:06 PM

Second try to upload patch with arcanist.

Harbormaster completed remote builds in B52763: Diff 256708.
This revision was automatically updated to reflect the committed changes.
aprantl added inline comments.Apr 13 2020, 12:22 PM
llvm/lib/Transforms/Utils/Debugify.cpp
203

I don't think there is, but this could become a helper in MDNode.h