--di with --opt keeps its locations, and inlined ones are made safe for the DI scope pass

tslang/include/TypeScript/Passes.h

@@ -48,6 +48,10 @@ std::unique_ptr<mlir::Pass> createOwnedReturnConsumptionPass(CompileOptions&);
 /// GC Pass to replace malloc, realloc, free with GC_malloc, GC_realloc, GC_free
 std::unique_ptr<mlir::Pass> createGCPass(CompileOptions&);
 std::unique_ptr<mlir::Pass> createProcessHeapPass();
+/// Makes the inliner's call-site locations safe for DIScopeForLLVMFuncOpPass: an inlined op with no
+/// location of its own takes the call's, and one outside any function (a string literal's global)
+/// the location it was written at. Schedule before it, under --di --opt.
+std::unique_ptr<mlir::Pass> createInlinedLocationPass();
 /// MemAlloc Pass to replace ts_malloc, ts_realloc, ts_free
 std::unique_ptr<mlir::Pass> createMemAllocPass(CompileOptions&);
 

tslang/lib/TypeScript/CMakeLists.txt

@@ -37,6 +37,7 @@ add_mlir_dialect_library(MLIRTypeScript
     OwnedReturnConsumptionPass.cpp
     GCPass.cpp
     ProcessHeapPass.cpp
+    InlinedLocationPass.cpp
     AsyncTargetWidthPass.cpp
     ObjDumper.cpp
     DeclarationPrinter.cpp

tslang/lib/TypeScript/InlinedLocationPass.cpp

@@ -0,0 +1,114 @@
+#include "mlir/Pass/Pass.h"
+
+#include "TypeScript/TypeScriptDialect.h"
+#include "TypeScript/TypeScriptOps.h"
+#include "TypeScript/Passes.h"
+#include "TypeScript/Pass/ModulePass.h"
+
+#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
+
+#include "llvm/Support/Debug.h"
+
+#define DEBUG_TYPE "pass"
+
+using namespace ::typescript;
+namespace mlir_ts = mlir::typescript;
+
+namespace
+{
+
+// Is there a file:line in this location, found the way DIScopeForLLVMFuncOpPass looks for one?
+static bool hasFileLoc(mlir::Location loc)
+{
+    if (isa<mlir::FileLineColLoc>(loc))
+    {
+        return true;
+    }
+
+    if (auto nameLoc = dyn_cast<mlir::NameLoc>(loc))
+    {
+        return hasFileLoc(nameLoc.getChildLoc());
+    }
+
+    if (auto opaqueLoc = dyn_cast<mlir::OpaqueLoc>(loc))
+    {
+        return hasFileLoc(opaqueLoc.getFallbackLocation());
+    }
+
+    if (auto fusedLoc = dyn_cast<mlir::FusedLoc>(loc))
+    {
+        return llvm::any_of(fusedLoc.getLocations(), hasFileLoc);
+    }
+
+    if (auto callSiteLoc = dyn_cast<mlir::CallSiteLoc>(loc))
+    {
+        return hasFileLoc(callSiteLoc.getCaller());
+    }
+
+    return false;
+}
+
+// An inlined op whose own location has no file:line stands for the call it was inlined from.
+static mlir::Location repair(mlir::Location loc)
+{
+    auto callSiteLoc = dyn_cast<mlir::CallSiteLoc>(loc);
+    if (!callSiteLoc)
+    {
+        return loc;
+    }
+
+    auto callee = repair(callSiteLoc.getCallee());
+    if (!hasFileLoc(callee))
+    {
+        return callSiteLoc.getCaller();
+    }
+
+    return callee == callSiteLoc.getCallee() ? loc : mlir::CallSiteLoc::get(callee, callSiteLoc.getCaller());
+}
+
+// Outside a function there is no inlining to describe: the location the op was written at.
+static mlir::Location written(mlir::Location loc)
+{
+    while (auto callSiteLoc = dyn_cast<mlir::CallSiteLoc>(loc))
+    {
+        loc = callSiteLoc.getCallee();
+    }
+
+    return loc;
+}
+
+// Under `--di --opt`, ahead of DIScopeForLLVMFuncOpPass. The inliner gives every op it moves
+// `callsite(<its location> at <the call>)`, and DIScopeForLLVMFuncOpPass takes two things for
+// granted about an op with such a location, both of which fail here and crashed tslang for most
+// programs:
+// - that its callee has a file name: an op made without a location of its own - there are some -
+//   becomes `callsite(unknown at <the call>)`;
+// - that it is inside a function: the lowering makes a string literal's global with the location
+//   of the op that used it, which is a call site when that op was inlined.
+class InlinedLocationPass : public mlir::PassWrapper<InlinedLocationPass, ModulePass>
+{
+  public:
+    MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(InlinedLocationPass)
+
+    void runOnModule() override
+    {
+        getModule().walk([](mlir::Operation *op) {
+            auto loc = op->getLoc();
+            auto repaired = isa<mlir::LLVM::LLVMFuncOp>(op) || op->getParentOfType<mlir::LLVM::LLVMFuncOp>()
+                                ? repair(loc)
+                                : written(loc);
+            if (repaired != loc)
+            {
+                op->setLoc(repaired);
+            }
+        });
+    }
+};
+} // end anonymous namespace
+
+#undef DEBUG_TYPE
+
+std::unique_ptr<mlir::Pass> mlir_ts::createInlinedLocationPass()
+{
+    return std::make_unique<InlinedLocationPass>();
+}

tslang/test/tester/CMakeLists.txt

@@ -2203,6 +2203,13 @@ add_test(NAME test-compile-rc-debug-info
                  -DTESTS_DIR=${PROJECT_SOURCE_DIR}/test/tester/tests
                  -P ${CMAKE_CURRENT_SOURCE_DIR}/debug-info-rc.cmake)
 
+# Debug info together with optimisation, under every model - see debug-info-opt.cmake.
+add_test(NAME test-compile-debug-info-opt
+         COMMAND ${CMAKE_COMMAND}
+                 -DTSLANG=$<TARGET_FILE:tslang>
+                 -DTESTS_DIR=${PROJECT_SOURCE_DIR}/test/tester/tests
+                 -P ${CMAKE_CURRENT_SOURCE_DIR}/debug-info-opt.cmake)
+
 # tslang choosing the collector's linkage itself under -mm=gc - see shared-collector-auto.cmake.
 if (WIN32)
     if ("${CMAKE_BUILD_TYPE}" STREQUAL "Release")

tslang/test/tester/debug-info-opt.cmake

@@ -0,0 +1,64 @@
+# Emits LLVM IR for a few programs WITH debug info AND optimisation, under every memory model.
+#
+# `test-runner` picks `--opt` or `--di` from the build configuration, never both, so nothing else
+# here asks for the pair - and it was broken three ways, all of them before any code was generated:
+#
+# - `--opt` stripped every location ahead of the inliner while `--di` still gave every function a
+#   subprogram, so a call the inliner left had no location in a function that needed one. Under rc
+#   that is every program (the `tsrel_`/`tsret_` helpers are made after the inliner runs): "inlinable
+#   function call in a function with a DISubprogram location must have a debug location".
+# - With the locations kept, DIScopeForLLVMFuncOpPass crashed on two shapes the inliner leaves: a
+#   call site whose callee has no file (an op made without a location, inlined), and a call site on
+#   an op outside any function (a string literal's global, made for an inlined op). 18enums.ts has
+#   both.
+#
+# A compile, not a run, for the same reason as debug-info-rc.cmake: every failure was a module that
+# would not come out.
+
+if(NOT DEFINED TSLANG OR NOT DEFINED TESTS_DIR)
+    message(FATAL_ERROR "TSLANG and TESTS_DIR are both required")
+endif()
+
+set(files
+    # inlined string literals and an op with no location of its own
+    18enums.ts
+    # a string local, an owning field, an array, a closure, a class
+    00owned_debug_info.ts
+    # classes, interfaces and closures, which reach the reference-counting helpers
+    00interface.ts
+    00owned_closures.ts
+    # the largest program here
+    raytrace.ts)
+
+set(failures "")
+set(count 0)
+foreach(model gc rc none)
+    foreach(name ${files})
+        execute_process(
+            COMMAND "${TSLANG}" --emit=llvm --di --opt -mm=${model} --no-default-lib
+                    "${TESTS_DIR}/${name}" -o=${name}.${model}.ll
+            OUTPUT_QUIET
+            ERROR_VARIABLE diagnostics
+            RESULT_VARIABLE status)
+
+        if(NOT status EQUAL 0)
+            # the first error, not a warning printed ahead of it
+            if(diagnostics MATCHES "[^\n]*error:[^\n]*")
+                set(first_line "${CMAKE_MATCH_0}")
+            else()
+                string(REGEX REPLACE "\n.*" "" first_line "${diagnostics}")
+            endif()
+            list(APPEND failures "  ${name} -mm=${model}: exit ${status}: ${first_line}")
+        endif()
+
+        file(REMOVE "${name}.${model}.ll")
+        math(EXPR count "${count} + 1")
+    endforeach()
+endforeach()
+
+if(failures)
+    string(REPLACE ";" "\n" report "${failures}")
+    message(FATAL_ERROR "debug info with --opt:\n${report}")
+endif()
+
+message(STATUS "debug info with --opt clean over ${count} compiles")

tslang/tslang/transform.cpp

@@ -153,7 +153,12 @@ int runMLIRPasses(mlir::MLIRContext &context, llvm::SourceMgr &sourceMgr, mlir::
         {
             optPM.addPass(mlir::createCSEPass());
             pm.addPass(mlir::createLoopInvariantCodeMotionPass());
-            pm.addPass(mlir::createStripDebugInfoPass());
+            // Not under --di: stripping left every function a subprogram and every call no
+            // location, which the LLVM dialect's verifier rejects for a call it could inline
+            if (!compileOptions.generateDebugInfo)
+            {
+                pm.addPass(mlir::createStripDebugInfoPass());
+            }
             pm.addPass(mlir::createInlinerPass());
             pm.addPass(mlir::createSCCPPass());
             pm.addPass(mlir::createSymbolDCEPass());
@@ -195,6 +200,11 @@ int runMLIRPasses(mlir::MLIRContext &context, llvm::SourceMgr &sourceMgr, mlir::
 #endif
         if (compileOptions.generateDebugInfo)
         {
+            if (enableOpt)
+            {
+                pm.addPass(mlir::typescript::createInlinedLocationPass());
+            }
+
             pm.addPass(mlir::LLVM::createDIScopeForLLVMFuncOpPass());
         }