aprantl updated this revision to Diff 176148.
aprantl added a reviewer: davide.
aprantl added a comment.
Herald added subscribers: nhaehnle, jvesely.
Turns out that my patch had an unintended interaction with the backend
diagnostics engine. This is an updated version of the patch that also updates
the backend diagnostics engine to still emit the same diagnostics as before
even with the more efficient DIFile encoding.
The backend diagnostics call back into the frontend to emit nicely formatted
diagnostics, but my initial CFE patch broke this mechanism for files with an
absolute path. This is now fixed by first looking up the relative path in the
FileManager and then falling back to the absolute path, which we construct by
joining the DIFile's directory and filename.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55085/new/
https://reviews.llvm.org/D55085
Files:
include/llvm/IR/DiagnosticInfo.h
lib/CodeGen/MachineLoopInfo.cpp
lib/IR/DiagnosticInfo.cpp
test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
tools/clang/lib/CodeGen/CGDebugInfo.cpp
tools/clang/lib/CodeGen/CodeGenAction.cpp
tools/clang/test/CodeGen/debug-info-abspath.c
tools/clang/test/CodeGen/debug-prefix-map.c
tools/clang/test/Modules/module-debuginfo-prefix.m
Index: test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
===================================================================
--- test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
+++ test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
@@ -17,7 +17,7 @@
!llvm.module.flags = !{!2, !3}
!0 = distinct !DICompileUnit(language: DW_LANG_OpenCL, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug)
-!1 = !DIFile(filename: "foo.cl", directory: "/dev/null")
+!1 = !DIFile(filename: "foo.cl", directory: ".")
!2 = !{i32 2, !"Dwarf Version", i32 4}
!3 = !{i32 2, !"Debug Info Version", i32 3}
!4 = !DILocation(line: 1, column: 42, scope: !5)
Index: lib/IR/DiagnosticInfo.cpp
===================================================================
--- lib/IR/DiagnosticInfo.cpp
+++ lib/IR/DiagnosticInfo.cpp
@@ -33,9 +33,10 @@
#include "llvm/Support/Casting.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Path.h"
#include "llvm/Support/Regex.h"
-#include "llvm/Support/raw_ostream.h"
#include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/raw_ostream.h"
#include <atomic>
#include <cassert>
#include <memory>
@@ -106,7 +107,7 @@
DiagnosticLocation::DiagnosticLocation(const DebugLoc &DL) {
if (!DL)
return;
- Filename = DL->getFilename();
+ File = DL->getFile();
Line = DL->getLine();
Column = DL->getColumn();
}
@@ -114,17 +115,32 @@
DiagnosticLocation::DiagnosticLocation(const DISubprogram *SP) {
if (!SP)
return;
- Filename = SP->getFilename();
+
+ File = SP->getFile();
Line = SP->getScopeLine();
Column = 0;
}
-void DiagnosticInfoWithLocationBase::getLocation(StringRef *Filename,
- unsigned *Line,
- unsigned *Column) const {
- *Filename = Loc.getFilename();
- *Line = Loc.getLine();
- *Column = Loc.getColumn();
+std::string DiagnosticLocation::getAbsolutePath() const {
+ StringRef Name = File->getFilename();
+ if (sys::path::is_absolute(Name))
+ return Name;
+
+ SmallString<128> Path;
+ sys::path::append(Path, File->getDirectory(), Name);
+ return sys::path::remove_leading_dotslash(Path).str();
+}
+
+std::string DiagnosticInfoWithLocationBase::getAbsolutePath() const {
+ return Loc.getAbsolutePath();
+}
+
+void DiagnosticInfoWithLocationBase::getLocation(StringRef &RelativePath,
+ unsigned &Line,
+ unsigned &Column) const {
+ RelativePath = Loc.getRelativePath();
+ Line = Loc.getLine();
+ Column = Loc.getColumn();
}
const std::string DiagnosticInfoWithLocationBase::getLocationStr() const {
@@ -132,7 +148,7 @@
unsigned Line = 0;
unsigned Column = 0;
if (isLocationAvailable())
- getLocation(&Filename, &Line, &Column);
+ getLocation(Filename, Line, Column);
return (Filename + ":" + Twine(Line) + ":" + Twine(Column)).str();
}
@@ -399,7 +415,7 @@
static void mapping(IO &io, DiagnosticLocation &DL) {
assert(io.outputting() && "input not yet implemented");
- StringRef File = DL.getFilename();
+ StringRef File = DL.getRelativePath();
unsigned Line = DL.getLine();
unsigned Col = DL.getColumn();
Index: lib/CodeGen/MachineLoopInfo.cpp
===================================================================
--- lib/CodeGen/MachineLoopInfo.cpp
+++ lib/CodeGen/MachineLoopInfo.cpp
@@ -19,6 +19,7 @@
#include "llvm/CodeGen/MachineDominators.h"
#include "llvm/CodeGen/Passes.h"
#include "llvm/Config/llvm-config.h"
+#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/raw_ostream.h"
using namespace llvm;
@@ -92,8 +93,10 @@
// Try the pre-header first.
if (MachineBasicBlock *PHeadMBB = getLoopPreheader())
if (const BasicBlock *PHeadBB = PHeadMBB->getBasicBlock())
- if (DebugLoc DL = PHeadBB->getTerminator()->getDebugLoc())
+ if (DebugLoc DL = PHeadBB->getTerminator()->getDebugLoc()) {
+ DL->dump();
return DL;
+ }
// If we have no pre-header or there are no instructions with debug
// info in it, try the header.
Index: include/llvm/IR/DiagnosticInfo.h
===================================================================
--- include/llvm/IR/DiagnosticInfo.h
+++ include/llvm/IR/DiagnosticInfo.h
@@ -340,7 +340,7 @@
};
class DiagnosticLocation {
- StringRef Filename;
+ DIFile *File = nullptr;
unsigned Line = 0;
unsigned Column = 0;
@@ -349,8 +349,11 @@
DiagnosticLocation(const DebugLoc &DL);
DiagnosticLocation(const DISubprogram *SP);
- bool isValid() const { return !Filename.empty(); }
- StringRef getFilename() const { return Filename; }
+ bool isValid() const { return File; }
+ /// Return the full path to the file.
+ std::string getAbsolutePath() const;
+ /// Return the file name relative to the compilation directory.
+ StringRef getRelativePath() const { return File->getFilename(); }
unsigned getLine() const { return Line; }
unsigned getColumn() const { return Column; }
};
@@ -375,9 +378,13 @@
const std::string getLocationStr() const;
/// Return location information for this diagnostic in three parts:
- /// the source file name, line number and column.
- void getLocation(StringRef *Filename, unsigned *Line, unsigned *Column) const;
+ /// the relative source file path, line number and column.
+ void getLocation(StringRef &RelativePath, unsigned &Line,
+ unsigned &Column) const;
+ /// Return the absolute path tot the file.
+ std::string getAbsolutePath() const;
+
const Function &getFunction() const { return Fn; }
DiagnosticLocation getLocation() const { return Loc; }
Index: tools/clang/test/Modules/module-debuginfo-prefix.m
===================================================================
--- tools/clang/test/Modules/module-debuginfo-prefix.m
+++ tools/clang/test/Modules/module-debuginfo-prefix.m
@@ -20,4 +20,4 @@
@import DebugObjC;
#endif
-// CHECK: !DIFile({{.*}}"/OVERRIDE/DebugObjC.h"
+// CHECK: !DIFile(filename: "/OVERRIDE/DebugObjC.h", directory: "")
Index: tools/clang/test/CodeGen/debug-prefix-map.c
===================================================================
--- tools/clang/test/CodeGen/debug-prefix-map.c
+++ tools/clang/test/CodeGen/debug-prefix-map.c
@@ -17,18 +17,22 @@
}
// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{/|\\5C}}<stdin>"
-// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}"
-// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{[/\\]}}Inputs/stdio.h"
+// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}",
+// CHECK-NO-MAIN-FILE-NAME-SAME: directory: "")
+// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{[/\\]}}Inputs/stdio.h",
+// CHECK-NO-MAIN-FILE-NAME-SAME: directory: "")
// CHECK-NO-MAIN-FILE-NAME-NOT: !DIFile(filename:
// CHECK-EVIL: !DIFile(filename: "/var=empty{{[/\\]}}{{.*}}"
-// CHECK-EVIL: !DIFile(filename: "/var=empty{{[/\\]}}Inputs/stdio.h"
+// CHECK-EVIL: !DIFile(filename: "/var=empty{{[/\\]}}{{.*}}Inputs/stdio.h",
+// CHECK-EVIL-SAME: directory: "")
// CHECK-EVIL-NOT: !DIFile(filename:
// CHECK: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}"
-// CHECK: !DIFile(filename: "/var/empty{{[/\\]}}Inputs/stdio.h"
+// CHECK: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}Inputs/stdio.h",
+// CHECK-SAME: directory: "")
// CHECK-NOT: !DIFile(filename:
-// CHECK-COMPILATION-DIR: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}", directory: "/var/empty")
-// CHECK-COMPILATION-DIR: !DIFile(filename: "/var/empty{{[/\\]}}Inputs/stdio.h", directory: "/var/empty")
+// CHECK-COMPILATION-DIR: !DIFile(filename: "{{.*}}", directory: "/var/empty")
+// CHECK-COMPILATION-DIR: !DIFile(filename: "Inputs/stdio.h", directory: "/var/empty")
// CHECK-COMPILATION-DIR-NOT: !DIFile(filename:
Index: tools/clang/test/CodeGen/debug-info-abspath.c
===================================================================
--- /dev/null
+++ tools/clang/test/CodeGen/debug-info-abspath.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN: %s -emit-llvm -o - | FileCheck %s
+
+// RUN: cp %s %t.c
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN: %t.c -emit-llvm -o - | FileCheck %s --check-prefix=INTREE
+void foo() {}
+
+// Since %s is an absolute path, directory should be a nonempty
+// prefix, but the CodeGen part should be part of the filename.
+
+// CHECK: DIFile(filename: "{{.*}}CodeGen{{.*}}debug-info-abspath.c"
+// CHECK-SAME: directory: "{{.+}}")
+
+// INTREE: DIFile({{.*}}directory: "{{.+}}CodeGen{{.*}}")
Index: tools/clang/lib/CodeGen/CodeGenAction.cpp
===================================================================
--- tools/clang/lib/CodeGen/CodeGenAction.cpp
+++ tools/clang/lib/CodeGen/CodeGenAction.cpp
@@ -549,12 +549,16 @@
SourceLocation DILoc;
if (D.isLocationAvailable()) {
- D.getLocation(&Filename, &Line, &Column);
- const FileEntry *FE = FileMgr.getFile(Filename);
- if (FE && Line > 0) {
- // If -gcolumn-info was not used, Column will be 0. This upsets the
- // source manager, so pass 1 if Column is not set.
- DILoc = SourceMgr.translateFileLineCol(FE, Line, Column ? Column : 1);
+ D.getLocation(Filename, Line, Column);
+ if (Line > 0) {
+ const FileEntry *FE = FileMgr.getFile(Filename);
+ if (!FE)
+ FE = FileMgr.getFile(D.getAbsolutePath());
+ if (FE) {
+ // If -gcolumn-info was not used, Column will be 0. This upsets the
+ // source manager, so pass 1 if Column is not set.
+ DILoc = SourceMgr.translateFileLineCol(FE, Line, Column ? Column : 1);
+ }
}
BadDebugInfo = DILoc.isInvalid();
}
Index: tools/clang/lib/CodeGen/CGDebugInfo.cpp
===================================================================
--- tools/clang/lib/CodeGen/CGDebugInfo.cpp
+++ tools/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -181,8 +181,7 @@
SourceManager &SM = CGM.getContext().getSourceManager();
auto *Scope = cast<llvm::DIScope>(LexicalBlockStack.back());
PresumedLoc PCLoc = SM.getPresumedLoc(CurLoc);
-
- if (PCLoc.isInvalid() || Scope->getFilename() == PCLoc.getFilename())
+ if (PCLoc.isInvalid() || Scope->getFile() == getOrCreateFile(CurLoc))
return;
if (auto *LBF = dyn_cast<llvm::DILexicalBlockFile>(Scope)) {
@@ -407,13 +406,13 @@
SourceManager &SM = CGM.getContext().getSourceManager();
PresumedLoc PLoc = SM.getPresumedLoc(Loc);
- if (PLoc.isInvalid() || StringRef(PLoc.getFilename()).empty())
+ StringRef FileName = PLoc.getFilename();
+ if (PLoc.isInvalid() || FileName.empty())
// If the location is not valid then use main input file.
return getOrCreateMainFile();
// Cache the results.
- const char *fname = PLoc.getFilename();
- auto It = DIFileCache.find(fname);
+ auto It = DIFileCache.find(FileName.data());
if (It != DIFileCache.end()) {
// Verify that the information still exists.
@@ -428,11 +427,41 @@
if (CSKind)
CSInfo.emplace(*CSKind, Checksum);
- llvm::DIFile *F = DBuilder.createFile(
- remapDIPath(PLoc.getFilename()), remapDIPath(getCurrentDirname()), CSInfo,
- getSource(SM, SM.getFileID(Loc)));
+ StringRef Dir;
+ StringRef File;
+ std::string RemappedFile = remapDIPath(FileName);
+ std::string CurDir = remapDIPath(getCurrentDirname());
+ SmallString<128> DirBuf;
+ SmallString<128> FileBuf;
+ if (llvm::sys::path::is_absolute(RemappedFile)) {
+ // Strip the common prefix (if it is more than just "/") from current
+ // directory and FileName for a more space-efficient encoding.
+ auto FileIt = llvm::sys::path::begin(RemappedFile);
+ auto FileE = llvm::sys::path::end(RemappedFile);
+ auto CurDirIt = llvm::sys::path::begin(CurDir);
+ auto CurDirE = llvm::sys::path::end(CurDir);
+ for (; CurDirIt != CurDirE && *CurDirIt == *FileIt; ++CurDirIt, ++FileIt)
+ llvm::sys::path::append(DirBuf, *CurDirIt);
+ if (std::distance(llvm::sys::path::begin(CurDir), CurDirIt) == 1) {
+ // The common prefix only the root; stripping it would cause
+ // LLVM diagnostic locations to be more confusing.
+ Dir = {};
+ File = RemappedFile;
+ } else {
+ for (; FileIt != FileE; ++FileIt)
+ llvm::sys::path::append(FileBuf, *FileIt);
+ Dir = DirBuf;
+ File = FileBuf;
+ }
+ } else {
+ Dir = CurDir;
+ File = RemappedFile;
+ }
+ llvm::DIFile *F =
+ DBuilder.createFile(File, Dir, CSInfo,
+ getSource(SM, SM.getFileID(Loc)));
- DIFileCache[fname].reset(F);
+ DIFileCache[FileName.data()].reset(F);
return F;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
