From f0506e4923cdbd2b53258bc6c3a2b6bc62c8ccc3 Mon Sep 17 00:00:00 2001
From: Tim Shen <timshen@google.com>
Date: Tue, 22 Sep 2020 23:36:08 -0700
Subject: [PATCH] [MLIR] Avoid adding debuginfo for a function if it contains
 calls that has no debug info.

Also add a verifier pass to ExecutionEngine.

It's hard to come up with a test case, since mlir-opt always add location info after parsing it (?)

Differential Revision: https://reviews.llvm.org/D88135
---
 mlir/lib/Target/LLVMIR/ConvertToLLVMIR.cpp  |  7 ++++++-
 mlir/lib/Target/LLVMIR/DebugTranslation.cpp | 12 ++++++++++++
 mlir/test/Target/llvmir-debug.mlir          |  4 ----
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/mlir/lib/Target/LLVMIR/ConvertToLLVMIR.cpp b/mlir/lib/Target/LLVMIR/ConvertToLLVMIR.cpp
index baf9b5e..cadd172 100644
--- a/mlir/lib/Target/LLVMIR/ConvertToLLVMIR.cpp
+++ b/mlir/lib/Target/LLVMIR/ConvertToLLVMIR.cpp
@@ -17,6 +17,7 @@
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/Verifier.h"
 #include "llvm/Support/ToolOutputFile.h"
 
 using namespace mlir;
@@ -24,7 +25,11 @@ using namespace mlir;
 std::unique_ptr<llvm::Module>
 mlir::translateModuleToLLVMIR(ModuleOp m, llvm::LLVMContext &llvmContext,
                               StringRef name) {
-  return LLVM::ModuleTranslation::translateModule<>(m, llvmContext, name);
+  auto llvmModule =
+      LLVM::ModuleTranslation::translateModule<>(m, llvmContext, name);
+  if (verifyModule(*llvmModule))
+    emitError(m.getLoc(), "LLVM IR fails to verify");
+  return llvmModule;
 }
 
 namespace mlir {
diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
index af364ba..a0a19a2 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
@@ -71,6 +71,18 @@ void DebugTranslation::translate(LLVMFuncOp func, llvm::Function &llvmFunc) {
   if (!compileUnit || !func.walk(interruptIfValidLocation).wasInterrupted())
     return;
 
+  // If we are to create debug info for the function, we need to ensure that all
+  // inlinable calls in it are with debug info, otherwise the LLVM verifier will
+  // complain. For now, be more restricted and treat all calls as inlinable.
+  const bool hasCallWithoutDebugInfo =
+      func.walk([](LLVM::CallOp call) {
+            return call.getLoc().isa<UnknownLoc>() ? WalkResult::interrupt()
+                                                   : WalkResult::advance();
+          })
+          .wasInterrupted();
+  if (hasCallWithoutDebugInfo)
+    return;
+
   FileLineColLoc fileLoc = extractFileLoc(func.getLoc());
   auto *file = translateFile(fileLoc ? fileLoc.getFilename() : "<unknown>");
   unsigned line = fileLoc ? fileLoc.getLine() : 0;
diff --git a/mlir/test/Target/llvmir-debug.mlir b/mlir/test/Target/llvmir-debug.mlir
index 2a94448..590fb8b 100644
--- a/mlir/test/Target/llvmir-debug.mlir
+++ b/mlir/test/Target/llvmir-debug.mlir
@@ -9,10 +9,6 @@ llvm.func @func_no_debug() {
 // CHECK-LABEL: define void @func_with_debug()
 // CHECK-SAME: !dbg ![[FUNC_LOC:[0-9]+]]
 llvm.func @func_with_debug() {
-  // CHECK: call void @func_no_debug()
-  // CHECK-NOT: !dbg
-  llvm.call @func_no_debug() : () -> () loc(unknown)
-
   // CHECK: call void @func_no_debug(), !dbg ![[CALLSITE_LOC:[0-9]+]]
   llvm.call @func_no_debug() : () -> () loc(callsite("mysource.cc":3:4 at "mysource.cc":5:6))
 
-- 
2.7.4