[llvm] 1adeeab - Add MIR-level debugify with only locations support for now
Daniel Sanders via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 7 16:25:28 PDT 2020
Author: Daniel Sanders
Date: 2020-04-07T16:25:13-07:00
New Revision: 1adeeabb79afbe966a45050f07e0bd6f446f8aa6
URL: https://github.com/llvm/llvm-project/commit/1adeeabb79afbe966a45050f07e0bd6f446f8aa6
DIFF: https://github.com/llvm/llvm-project/commit/1adeeabb79afbe966a45050f07e0bd6f446f8aa6.diff
LOG: Add MIR-level debugify with only locations support for now
Summary:
Re-used the IR-level debugify for the most part. The MIR-level code then
adds locations to the MachineInstrs afterwards based on the LLVM-IR debug
info.
It's worth mentioning that the resulting locations make little sense as
the range of line numbers used in a Function at the MIR level exceeds that
of the equivelent IR level function. As such, MachineInstrs can appear to
originate from outside the subprogram scope (and from other subprogram
scopes). However, it doesn't seem worth worrying about as the source is
imaginary anyway.
There's a few high level goals this pass works towards:
* We should be able to debugify our .ll/.mir in the lit tests without
changing the checks and still pass them. I.e. Debug info should not change
codegen. Combining this with a strip-debug pass should enable this. The
main issue I ran into without the strip-debug pass was instructions with MMO's and
checks on both the instruction and the MMO as the debug-location is
between them. I currently have a simple hack in the MIRPrinter to
resolve that but the more general solution is a proper strip-debug pass.
* We should be able to test that GlobalISel does not lose debug info. I
recently found that the legalizer can be unexpectedly lossy in seemingly
simple cases (e.g. expanding one instr into many). I have a verifier
(will be posted separately) that can be integrated with passes that use
the observer interface and will catch location loss (it does not verify
correctness, just that there's zero lossage). It is a little conservative
as the line-0 locations that arise from conflicts do not track the
conflicting locations but it can still catch a fair bit.
Depends on D77439, D77438
Reviewers: aprantl, bogner, vsk
Subscribers: mgorny, hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D77446
Added:
llvm/lib/CodeGen/MachineDebugify.cpp
llvm/test/CodeGen/Generic/MIRDebugify/locations.mir
Modified:
llvm/include/llvm/CodeGen/Passes.h
llvm/include/llvm/InitializePasses.h
llvm/include/llvm/Transforms/Utils/Debugify.h
llvm/lib/CodeGen/CMakeLists.txt
llvm/lib/CodeGen/CodeGen.cpp
llvm/lib/Transforms/Utils/Debugify.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/Passes.h b/llvm/include/llvm/CodeGen/Passes.h
index 997f36eb6aed..7627baa2f059 100644
--- a/llvm/include/llvm/CodeGen/Passes.h
+++ b/llvm/include/llvm/CodeGen/Passes.h
@@ -473,6 +473,8 @@ namespace llvm {
/// Create IR Type Promotion pass. \see TypePromotion.cpp
FunctionPass *createTypePromotionPass();
+ /// Creates MIR Debugify pass. \see MachineDebugify.cpp
+ ModulePass *createDebugifyMachineModulePass();
} // End llvm namespace
#endif
diff --git a/llvm/include/llvm/InitializePasses.h b/llvm/include/llvm/InitializePasses.h
index 263fa87f6b75..a71079fdc2b5 100644
--- a/llvm/include/llvm/InitializePasses.h
+++ b/llvm/include/llvm/InitializePasses.h
@@ -121,6 +121,7 @@ void initializeDSELegacyPassPass(PassRegistry&);
void initializeDataFlowSanitizerPass(PassRegistry&);
void initializeDeadInstEliminationPass(PassRegistry&);
void initializeDeadMachineInstructionElimPass(PassRegistry&);
+void initializeDebugifyMachineModulePass(PassRegistry &);
void initializeDelinearizationPass(PassRegistry&);
void initializeDemandedBitsWrapperPassPass(PassRegistry&);
void initializeDependenceAnalysisPass(PassRegistry&);
diff --git a/llvm/include/llvm/Transforms/Utils/Debugify.h b/llvm/include/llvm/Transforms/Utils/Debugify.h
index 0b5ec738750d..aed6fb5e6da2 100644
--- a/llvm/include/llvm/Transforms/Utils/Debugify.h
+++ b/llvm/include/llvm/Transforms/Utils/Debugify.h
@@ -17,6 +17,22 @@
#include "llvm/ADT/MapVector.h"
#include "llvm/IR/PassManager.h"
+namespace llvm {
+class DIBuilder;
+
+/// Add synthesized debug information to a module.
+///
+/// \param M The module to add debug information to.
+/// \param Functions A range of functions to add debug information to.
+/// \param Banner A prefix string to add to debug/error messages.
+/// \param ApplyToMF A call back that will add debug information to the
+/// MachineFunction for a Function. If nullptr, then the
+/// MachineFunction (if any) will not be modified.
+bool applyDebugifyMetadata(
+ Module &M, iterator_range<Module::iterator> Functions, StringRef Banner,
+ std::function<bool(DIBuilder &, Function &)> ApplyToMF);
+} // namespace llvm
+
llvm::ModulePass *createDebugifyModulePass();
llvm::FunctionPass *createDebugifyFunctionPass();
diff --git a/llvm/lib/CodeGen/CMakeLists.txt b/llvm/lib/CodeGen/CMakeLists.txt
index 5113b488f2c0..38187897fb66 100644
--- a/llvm/lib/CodeGen/CMakeLists.txt
+++ b/llvm/lib/CodeGen/CMakeLists.txt
@@ -73,6 +73,7 @@ add_llvm_component_library(LLVMCodeGen
MachineCombiner.cpp
MachineCopyPropagation.cpp
MachineCSE.cpp
+ MachineDebugify.cpp
MachineDominanceFrontier.cpp
MachineDominators.cpp
MachineFrameInfo.cpp
diff --git a/llvm/lib/CodeGen/CodeGen.cpp b/llvm/lib/CodeGen/CodeGen.cpp
index eefb328c9c60..f3b596a8f2bb 100644
--- a/llvm/lib/CodeGen/CodeGen.cpp
+++ b/llvm/lib/CodeGen/CodeGen.cpp
@@ -27,6 +27,7 @@ void llvm::initializeCodeGen(PassRegistry &Registry) {
initializeCFIInstrInserterPass(Registry);
initializeCodeGenPreparePass(Registry);
initializeDeadMachineInstructionElimPass(Registry);
+ initializeDebugifyMachineModulePass(Registry);
initializeDetectDeadLanesPass(Registry);
initializeDwarfEHPreparePass(Registry);
initializeEarlyIfConverterPass(Registry);
diff --git a/llvm/lib/CodeGen/MachineDebugify.cpp b/llvm/lib/CodeGen/MachineDebugify.cpp
new file mode 100644
index 000000000000..86834dbc53d5
--- /dev/null
+++ b/llvm/lib/CodeGen/MachineDebugify.cpp
@@ -0,0 +1,84 @@
+//===- MachineDebugify.cpp - Attach synthetic debug info to everything ----===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file This pass attaches synthetic debug info to everything. It can be used
+/// to create targeted tests for debug info preservation.
+///
+/// This isn't intended to have feature parity with Debugify.
+//===----------------------------------------------------------------------===//
+
+#include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
+#include "llvm/CodeGen/Passes.h"
+#include "llvm/IR/DIBuilder.h"
+#include "llvm/IR/DebugInfo.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Transforms/Utils/Debugify.h"
+
+#define DEBUG_TYPE "mir-debugify"
+
+using namespace llvm;
+
+namespace {
+bool applyDebugifyMetadataToMachineFunction(MachineModuleInfo &MMI,
+ DIBuilder &DIB, Function &F) {
+ MachineFunction &MF = MMI.getOrCreateMachineFunction(F);
+
+ DISubprogram *SP = F.getSubprogram();
+ assert(SP && "IR Debugify just created it?");
+
+ LLVMContext &Ctx = F.getParent()->getContext();
+ unsigned NextLine = SP->getLine();
+
+ for (MachineBasicBlock &MBB : MF) {
+ for (MachineInstr &MI : MBB) {
+ // This will likely emit line numbers beyond the end of the imagined
+ // source function and into subsequent ones. We don't do anything about
+ // that as it doesn't really matter to the compiler where the line is in
+ // the imaginary source code.
+ MI.setDebugLoc(DILocation::get(Ctx, NextLine++, 1, SP));
+ }
+ }
+
+ return true;
+}
+
+/// ModulePass for attaching synthetic debug info to everything, used with the
+/// legacy module pass manager.
+struct DebugifyMachineModule : public ModulePass {
+ bool runOnModule(Module &M) override {
+ MachineModuleInfo &MMI =
+ getAnalysis<MachineModuleInfoWrapperPass>().getMMI();
+ return applyDebugifyMetadata(
+ M, M.functions(),
+ "ModuleDebugify: ", [&](DIBuilder &DIB, Function &F) -> bool {
+ return applyDebugifyMetadataToMachineFunction(MMI, DIB, F);
+ });
+ }
+
+ DebugifyMachineModule() : ModulePass(ID) {}
+
+ void getAnalysisUsage(AnalysisUsage &AU) const override {
+ AU.addRequired<MachineModuleInfoWrapperPass>();
+ AU.addPreserved<MachineModuleInfoWrapperPass>();
+ }
+
+ static char ID; // Pass identification.
+};
+char DebugifyMachineModule::ID = 0;
+
+} // end anonymous namespace
+
+INITIALIZE_PASS_BEGIN(DebugifyMachineModule, DEBUG_TYPE,
+ "Machine Debugify Module", false, false)
+INITIALIZE_PASS_END(DebugifyMachineModule, DEBUG_TYPE,
+ "Machine Debugify Module", false, false)
+
+ModulePass *createDebugifyMachineModulePass() {
+ return new DebugifyMachineModule();
+}
diff --git a/llvm/lib/Transforms/Utils/Debugify.cpp b/llvm/lib/Transforms/Utils/Debugify.cpp
index a9cdf652b271..f32cdbc80e09 100644
--- a/llvm/lib/Transforms/Utils/Debugify.cpp
+++ b/llvm/lib/Transforms/Utils/Debugify.cpp
@@ -62,10 +62,11 @@ Instruction *findTerminatingInstruction(BasicBlock &BB) {
return I;
return BB.getTerminator();
}
+} // end anonymous namespace
-bool applyDebugifyMetadata(Module &M,
- iterator_range<Module::iterator> Functions,
- StringRef Banner) {
+bool llvm::applyDebugifyMetadata(
+ Module &M, iterator_range<Module::iterator> Functions, StringRef Banner,
+ std::function<bool(DIBuilder &DIB, Function &F)> ApplyToMF) {
// Skip modules with debug info.
if (M.getNamedMetadata("llvm.dbg.cu")) {
dbg() << Banner << "Skipping module with debug info\n";
@@ -149,6 +150,8 @@ bool applyDebugifyMetadata(Module &M,
InsertBefore);
}
}
+ if (ApplyToMF)
+ ApplyToMF(DIB, F);
DIB.finalizeSubprogram(SP);
}
DIB.finalize();
@@ -173,6 +176,7 @@ bool applyDebugifyMetadata(Module &M,
return true;
}
+namespace {
/// Return true if a mis-sized diagnostic is issued for \p DVI.
bool diagnoseMisSizedDbgValue(Module &M, DbgValueInst *DVI) {
// The size of a dbg.value's value operand should match the size of the
@@ -315,7 +319,8 @@ bool checkDebugifyMetadata(Module &M,
/// legacy module pass manager.
struct DebugifyModulePass : public ModulePass {
bool runOnModule(Module &M) override {
- return applyDebugifyMetadata(M, M.functions(), "ModuleDebugify: ");
+ return applyDebugifyMetadata(M, M.functions(),
+ "ModuleDebugify: ", /*ApplyToMF*/ nullptr);
}
DebugifyModulePass() : ModulePass(ID) {}
@@ -334,7 +339,7 @@ struct DebugifyFunctionPass : public FunctionPass {
Module &M = *F.getParent();
auto FuncIt = F.getIterator();
return applyDebugifyMetadata(M, make_range(FuncIt, std::next(FuncIt)),
- "FunctionDebugify: ");
+ "FunctionDebugify: ", /*ApplyToMF*/ nullptr);
}
DebugifyFunctionPass() : FunctionPass(ID) {}
@@ -409,7 +414,8 @@ FunctionPass *createDebugifyFunctionPass() {
}
PreservedAnalyses NewPMDebugifyPass::run(Module &M, ModuleAnalysisManager &) {
- applyDebugifyMetadata(M, M.functions(), "ModuleDebugify: ");
+ applyDebugifyMetadata(M, M.functions(),
+ "ModuleDebugify: ", /*ApplyToMF*/ nullptr);
return PreservedAnalyses::all();
}
diff --git a/llvm/test/CodeGen/Generic/MIRDebugify/locations.mir b/llvm/test/CodeGen/Generic/MIRDebugify/locations.mir
new file mode 100644
index 000000000000..17aab12a6fef
--- /dev/null
+++ b/llvm/test/CodeGen/Generic/MIRDebugify/locations.mir
@@ -0,0 +1,37 @@
+# RUN: llc -run-pass=mir-debugify -o - %s | FileCheck --check-prefixes=ALL,VALUE %s
+# RUN: llc -run-pass=mir-debugify -debugify-level=locations -o - %s | FileCheck --check-prefixes=ALL --implicit-check-not=dbg.value %s
+--- |
+ ; ModuleID = 'loc-only.ll'
+ source_filename = "loc-only.ll"
+
+ ; ALL-LABEL: @test
+ define i32 @test(i32 %a, i32 %b) {
+ %add = add i32 %a, 2
+ ; ALL-NEXT: %add = add i32 %a, 2, !dbg [[L1:![0-9]+]]
+ ; VALUE-NEXT: call void @llvm.dbg.value(metadata i32 %add, metadata [[add:![0-9]+]], metadata !DIExpression()), !dbg [[L1]]
+ %sub = sub i32 %add, %b
+ ; ALL-NEXT: %sub = sub i32 %add, %b, !dbg [[L2:![0-9]+]]
+ ; VALUE-NEXT: call void @llvm.dbg.value(metadata i32 %sub, metadata [[sub:![0-9]+]], metadata !DIExpression()), !dbg [[L2]]
+ ; ALL-NEXT: ret i32 %sub, !dbg [[L3:![0-9]+]]
+ ret i32 %sub
+ }
+
+...
+---
+name: test
+body: |
+ bb.1 (%ir-block.0):
+ %0:_(s32) = IMPLICIT_DEF
+ %1:_(s32) = IMPLICIT_DEF
+ %2:_(s32) = G_CONSTANT i32 2
+ %3:_(s32) = G_ADD %0, %2
+ %4:_(s32) = G_SUB %3, %1
+ ; There's no attempt to have the locations make sense as it's an imaginary
+ ; source file anyway. These first three coincide with IR-level information
+ ; and therefore use metadata references.
+ ; ALL: %0:_(s32) = IMPLICIT_DEF debug-location [[L1]]
+ ; ALL: %1:_(s32) = IMPLICIT_DEF debug-location [[L2]]
+ ; ALL: %2:_(s32) = G_CONSTANT i32 2, debug-location [[L3]]
+ ; ALL: %3:_(s32) = G_ADD %0, %2, debug-location !DILocation(line: 4, column: 1, scope: !6)
+ ; ALL: %4:_(s32) = G_SUB %3, %1, debug-location !DILocation(line: 5, column: 1, scope: !6)
+...
More information about the llvm-commits
mailing list