[Flang][Debug] Use pathnames from location of functions

[kiranchandramohan]

Summary

This ensures that functions in included files have the correct path
in their file metadata.

Note: This patch also sets all locations to have the full path names.

[kiranchandramohan]

Add comments in test files.

[kiranchandramohan]

Address comments from @vzakhari/@jeanPerier in D137956.

flang/lib/Lower/Bridge.cpp

714

Note: This change will convert all paths to absolute paths in location information.

[PeteSteinfeld]

All builds and tests correctly and looks good.

[vzakhari]

flang/lib/Lower/Bridge.cpp

712

I see that clang checks for - input in some case before trying to make the path absolute - I wonder if we need to handle the stdin here.

flang/lib/Optimizer/Transforms/AddDebugFoundation.cpp

63

nit: I think the additional hashing is excessive, because the attributes should be hashed already.

flang/test/Transforms/debug-line-table-inc-file.fir

29

nit: as written, [[FILE]] and [[INC_FILE]] may match to the same file name. A more robust checking criteria should be using parts of the original file names.

[kiranchandramohan]

Address comments from @vzakhari.

[kiranchandramohan]

flang/lib/Lower/Bridge.cpp

712

Thanks for bringing up this point. I think we need some special handling for stdin. I would have to look into this in a bit more detail before handling it. Would it be OK to handle this in a follow-up patch? I can create a ticket for this.

I see the following:
-> Clang seems to have both - and <stdin>.
-> MLIR seems to be using <stdin>.
-> Flang frontend seems to be passing standard input as filename.
-> Gfortran uses <stdin>

flang/lib/Optimizer/Transforms/AddDebugFoundation.cpp

63

Good point.

flang/test/Transforms/debug-line-table-inc-file.fir

29

Using the file name now.

[vzakhari]

Thank you for the updates! LGTM

flang/lib/Lower/Bridge.cpp

712

Thank you for looking it up, Kiran! Yes, it is okay to handle it seprately. A ticket would be good.

flang/lib/Lower/Bridge.cpp

public:
   genLocation(const Fortran::parser::CharBlock &block) override final {
     if (const Fortran::parser::AllCookedSources *cooked =
             bridge.getCookedSource()) {
       if (std::optional<std::pair<Fortran::parser::SourcePosition,
                                   Fortran::parser::SourcePosition>>
               loc = cooked->GetSourcePositionRange(block)) {
         // loc is a pair (begin, end); use the beginning position
         Fortran::parser::SourcePosition &filePos = loc->first;
-        return mlir::FileLineColLoc::get(&getMLIRContext(), filePos.file.path(),
+        llvm::SmallString<256> filePath(filePos.file.path());
+        llvm::sys::fs::make_absolute(filePath);
+        llvm::sys::path::remove_dots(filePath);
+        return mlir::FileLineColLoc::get(&getMLIRContext(), filePath.str(),
                                          filePos.line, filePos.column);
       }
     }
     return genUnknownLocation();
   }
 
   fir::FirOpBuilder &getFirOpBuilder() override final { return *builder; }
 

flang/lib/Optimizer/Transforms/AddDebugFoundation.cpp

 void AddDebugFoundationPass::runOnOperation() {
   mlir::ModuleOp module = getOperation();
   mlir::MLIRContext *context = &getContext();
   mlir::OpBuilder builder(context);
   std::string inputFilePath("-");
   if (auto fileLoc = module.getLoc().dyn_cast<mlir::FileLineColLoc>())
     inputFilePath = fileLoc.getFilename().getValue();
 
-  mlir::LLVM::DIFileAttr fileAttr = mlir::LLVM::DIFileAttr::get(
-      context, llvm::sys::path::filename(inputFilePath),
-      llvm::sys::path::parent_path(inputFilePath));
+  auto getFileAttr = [context](llvm::StringRef path) -> mlir::LLVM::DIFileAttr {
+    return mlir::LLVM::DIFileAttr::get(context, llvm::sys::path::filename(path),
+                                       llvm::sys::path::parent_path(path));
+  };
+
+  mlir::LLVM::DIFileAttr fileAttr = getFileAttr(inputFilePath);
   mlir::StringAttr producer = mlir::StringAttr::get(context, "Flang");
   mlir::LLVM::DICompileUnitAttr cuAttr = mlir::LLVM::DICompileUnitAttr::get(
       context, llvm::dwarf::getLanguage("DW_LANG_Fortran95"), fileAttr,
       producer, /*isOptimized=*/false,
       mlir::LLVM::DIEmissionKind::LineTablesOnly);
+
   module.walk([&](mlir::func::FuncOp funcOp) {
+    mlir::Location l = funcOp->getLoc();
+    // If fused location has already been created then nothing to do
+    // Otherwise, create a fused location.
+    if (l.dyn_cast<mlir::FusedLoc>())
+      return;
+
+    llvm::StringRef funcFilePath;
+    if (l.dyn_cast<mlir::FileLineColLoc>())
+      funcFilePath =
+          l.dyn_cast<mlir::FileLineColLoc>().getFilename().getValue();
+    else
+      funcFilePath = inputFilePath;
+
     mlir::StringAttr funcName =
         mlir::StringAttr::get(context, funcOp.getName());
     mlir::LLVM::DIBasicTypeAttr bT = mlir::LLVM::DIBasicTypeAttr::get(
         context, llvm::dwarf::DW_TAG_base_type, "void", /*sizeInBits=*/0,
         /*encoding=*/1);
     mlir::LLVM::DISubroutineTypeAttr subTypeAttr =
         mlir::LLVM::DISubroutineTypeAttr::get(
             context, llvm::dwarf::getCallingConvention("DW_CC_normal"),
             {bT, bT});
+    mlir::LLVM::DIFileAttr funcFileAttr = getFileAttr(funcFilePath);
     mlir::LLVM::DISubprogramAttr spAttr = mlir::LLVM::DISubprogramAttr::get(
-        context, cuAttr, fileAttr, funcName, funcName, fileAttr, /*line=*/1,
+        context, cuAttr, fileAttr, funcName, funcName, funcFileAttr, /*line=*/1,
         /*scopeline=*/1, mlir::LLVM::DISubprogramFlags::Definition,
         subTypeAttr);
     funcOp->setLoc(builder.getFusedLoc({funcOp->getLoc()}, spAttr));
   });
 }
 
 std::unique_ptr<mlir::Pass> fir::createAddDebugFoundationPass() {
   return std::make_unique<AddDebugFoundationPass>();
 }

flang/test/Transforms/debug-line-table-existing.fir

+
+// RUN: fir-opt --add-debug-foundation --mlir-print-debuginfo %s | FileCheck %s
+// REQUIRES: linux
+
+// Test that there are no changes to a function with existed fused loc debug
+module {
+  func.func @_QPs1() {
+    return loc(#loc1)
+  } loc(#loc2)
+} loc(#loc)
+#di_basic_type = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "void", encoding = DW_ATE_address>
+#di_file = #llvm.di_file<"simple.f90" in "/home/user01/llvm-project/build_release">
+#loc = loc("/home/user01/llvm-project/build_release/simple.f90":0:0)
+#loc1 = loc("/home/user01/llvm-project/build_release/simple.f90":1:1)
+#di_compile_unit = #llvm.di_compile_unit<sourceLanguage = DW_LANG_Fortran95, file = #di_file, producer = "Flang", isOptimized = false, emissionKind = LineTablesOnly>
+#di_subroutine_type = #llvm.di_subroutine_type<callingConvention = DW_CC_normal, types = #di_basic_type, #di_basic_type>
+#di_subprogram = #llvm.di_subprogram<compileUnit = #di_compile_unit, scope = #di_file, name = "_QPs1", linkageName = "_QPs1", file = #di_file, line = 1, scopeLine = 1, subprogramFlags = Definition, type = #di_subroutine_type>
+#loc2 = loc(fused<#di_subprogram>[#loc1])
+
+module {
+  func.func @_QPs1() {
+    return loc(#loc1)
+  } loc(#loc2)
+} loc(#loc)
+// CHECK: #loc = loc("/home/user01/llvm-project/build_release/simple.f90":0:0)
+// CHECK: #loc1 = loc("/home/user01/llvm-project/build_release/simple.f90":1:1)
+// CHECK: #di_subprogram = #llvm.di_subprogram<compileUnit = #di_compile_unit, scope = #di_file, name = "_QPs1", linkageName = "_QPs1", file = #di_file, line = 1, scopeLine = 1, subprogramFlags = Definition, type = #di_subroutine_type>
+// CHECK: #loc2 = loc(fused<#di_subprogram>[#loc1])

flang/test/Transforms/debug-line-table-inc-file.fir

+
+// RUN: fir-opt --add-debug-foundation --mlir-print-debuginfo %s | FileCheck %s
+// REQUIRES: linux
+
+// Test for included functions that have a different debug location than the current file
+module attributes {} {
+  func.func @_QPsinc() {
+    return loc(#loc2)
+  } loc(#loc1)
+  func.func @_QQmain() {
+    fir.call @_QPsinc() fastmath<contract> : () -> () loc(#loc4)
+    return loc(#loc5)
+  } loc(#loc3)
+} loc(#loc)
+#loc = loc("/home/user01/llvm-project/build_release/simple.f90":0:0)
+#loc1 = loc("/home/user01/llvm-project/build_release/inc.f90":1:1)
+#loc2 = loc("/home/user01/llvm-project/build_release/inc.f90":2:1)
+#loc3 = loc("/home/user01/llvm-project/build_release/simple.f90":3:1)
+#loc4 = loc("/home/user01/llvm-project/build_release/simple.f90":4:3)
+#loc5 = loc("/home/user01/llvm-project/build_release/simple.f90":5:1)
+
+// CHECK: module {
+// CHECK:   func.func @_QPsinc() {
+// CHECK:   } loc(#[[FUSED_LOC_INC_FILE:.*]])
+// CHECK:   func.func @_QQmain() {
+// CHECK:   } loc(#[[FUSED_LOC_FILE:.*]])
+// CHECK: } loc(#[[MODULE_LOC:.*]])
+// CHECK: #[[DI_FILE:.*]] = #llvm.di_file<"simple.f90" in "[[DIR:.*]]">
+// CHECK: #[[DI_INC_FILE:.*]] = #llvm.di_file<"inc.f90" in "[[DIR]]">
+// CHECK: #[[MODULE_LOC]] = loc("{{.*}}simple.f90":0:0)
+// CHECK: #[[LOC_INC_FILE:.*]] = loc("{{.*}}inc.f90":1:1)
+// CHECK: #[[LOC_FILE:.*]] = loc("{{.*}}simple.f90":3:1)
+// CHECK: #[[DI_CU:.*]] = #llvm.di_compile_unit<sourceLanguage = DW_LANG_Fortran95, file = #[[DI_FILE]], producer = "Flang", isOptimized = false, emissionKind = LineTablesOnly>
+// CHECK: #[[DI_SP_INC:.*]] = #llvm.di_subprogram<compileUnit = #[[DI_CU]], scope = #[[DI_FILE]], name = "_QPsinc", linkageName = "_QPsinc", file = #[[DI_INC_FILE]], {{.*}}>
+// CHECK: #[[DI_SP:.*]] = #llvm.di_subprogram<compileUnit = #[[DI_CU]], scope = #[[DI_FILE]], name = "_QQmain", linkageName = "_QQmain", file = #[[DI_FILE]], {{.*}}>
+// CHECK: #[[FUSED_LOC_INC_FILE]] = loc(fused<#[[DI_SP_INC]]>[#[[LOC_INC_FILE]]])
+// CHECK: #[[FUSED_LOC_FILE]] = loc(fused<#[[DI_SP]]>[#[[LOC_FILE]]])

flang/test/Transforms/debug-line-table-inc-same-file.fir

+
+// RUN: fir-opt --add-debug-foundation --mlir-print-debuginfo %s | FileCheck %s
+// REQUIRES: linux
+
+// Test that there is only one FileAttribute generated for multiple functions
+// in the same file.
+module attributes {} {
+  func.func @_QPs1() {
+    return loc(#loc2)
+  } loc(#loc1)
+  func.func @_QPs2() {
+    return loc(#loc2)
+  } loc(#loc1)
+  func.func @_QQmain() {
+    return loc(#loc3)
+  } loc(#loc1)
+} loc(#loc)
+#loc = loc("/home/user01/llvm-project/build_release/simple.f90":0:0)
+#loc1 = loc("/home/user01/llvm-project/build_release/simple.f90":1:1)
+#loc2 = loc("/home/user01/llvm-project/build_release/simple.f90":2:1)
+#loc3 = loc("/home/user01/llvm-project/build_release/simple.f90":3:1)
+
+// CHECK-COUNT-1: #llvm.di_file