[clang] Add a CodeGen option to ignore compilation directories #149897
Conversation
Created using spr 1.3.6
llvmbot
added
clang
cachemeifyoucan
requested review from
adrian-prantl,
benlangmuir and
jansvoboda11
|
@llvm/pr-subscribers-debuginfo @llvm/pr-subscribers-clang-codegen Author: Steven Wu (cachemeifyoucan) ChangesWhen enabling explicit gmodule builds, it is hard to enable unused Add an new cc1 flag Full diff: https://github.com/llvm/llvm-project/pull/149897.diff 8 Files Affected:
diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index cdeedd5b4eac6..9ec2784f6ce1c 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -231,6 +231,9 @@ class CodeGenOptions : public CodeGenOptionsBase {
/// The string to embed in coverage mapping as the current working directory.
std::string CoverageCompilationDir;
+ /// No compilation directory, ignore debug/coverage compilation directory.
+ bool NoCompilationDir;
+
/// The string to embed in the debug information for the compile unit, if
/// non-empty.
std::string DwarfDebugFlags;
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index e30c152cbce2e..1307e8a6e9e12 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1725,6 +1725,11 @@ def fcoverage_compilation_dir_EQ : Joined<["-"], "fcoverage-compilation-dir=">,
def ffile_compilation_dir_EQ : Joined<["-"], "ffile-compilation-dir=">, Group<f_Group>,
Visibility<[ClangOption, CLOption, DXCOption]>,
HelpText<"The compilation directory to embed in the debug info and coverage mapping.">;
+def fno_compilation_dir: Flag<["-"], "fno-compilation-dir">,
+ Visibility<[ClangOption, CC1Option]>,
+ Group<f_Group>,
+ HelpText<"Ignore compilation directories">,
+ MarshallingInfoFlag<CodeGenOpts<"NoCompilationDir">>;
defm debug_info_for_profiling : BoolFOption<"debug-info-for-profiling",
CodeGenOpts<"DebugInfoForProfiling">, DefaultFalse,
PosFlag<SetTrue, [], [ClangOption, CC1Option],
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index a371b6755f74d..8e4644e594d71 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -643,6 +643,9 @@ unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) {
}
StringRef CGDebugInfo::getCurrentDirname() {
+ if (CGM.getCodeGenOpts().NoCompilationDir)
+ return StringRef();
+
if (!CGM.getCodeGenOpts().DebugCompilationDir.empty())
return CGM.getCodeGenOpts().DebugCompilationDir;
@@ -3246,6 +3249,9 @@ llvm::DIModule *CGDebugInfo::getOrCreateModuleRef(ASTSourceDescriptor Mod,
std::string Remapped = remapDIPath(Path);
StringRef Relative(Remapped);
StringRef CompDir = TheCU->getDirectory();
+ if (CompDir.empty())
+ return Remapped;
+
if (Relative.consume_front(CompDir))
Relative.consume_front(llvm::sys::path::get_separator());
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 4aafac349e3e9..0e90908c8db7b 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -2449,6 +2449,9 @@ CoverageMappingModuleGen::CoverageMappingModuleGen(
: CGM(CGM), SourceInfo(SourceInfo) {}
std::string CoverageMappingModuleGen::getCurrentDirname() {
+ if (CGM.getCodeGenOpts().NoCompilationDir)
+ return {};
+
if (!CGM.getCodeGenOpts().CoverageCompilationDir.empty())
return CGM.getCodeGenOpts().CoverageCompilationDir;
diff --git a/clang/lib/CodeGen/ObjectFilePCHContainerWriter.cpp b/clang/lib/CodeGen/ObjectFilePCHContainerWriter.cpp
index 95971e57086e7..a5be5bbee753b 100644
--- a/clang/lib/CodeGen/ObjectFilePCHContainerWriter.cpp
+++ b/clang/lib/CodeGen/ObjectFilePCHContainerWriter.cpp
@@ -164,6 +164,8 @@ class PCHContainerGenerator : public ASTConsumer {
CodeGenOpts.DwarfVersion = CI.getCodeGenOpts().DwarfVersion;
CodeGenOpts.DebugCompilationDir =
CI.getInvocation().getCodeGenOpts().DebugCompilationDir;
+ CodeGenOpts.NoCompilationDir =
+ CI.getInvocation().getCodeGenOpts().NoCompilationDir;
CodeGenOpts.DebugPrefixMap =
CI.getInvocation().getCodeGenOpts().DebugPrefixMap;
CodeGenOpts.DebugStrictDwarf = CI.getCodeGenOpts().DebugStrictDwarf;
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 37f8b945d785e..7226e06db9092 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -144,30 +144,9 @@ static void optimizeDiagnosticOpts(DiagnosticOptions &Opts,
static void optimizeCWD(CowCompilerInvocation &BuildInvocation, StringRef CWD) {
BuildInvocation.getMutFileSystemOpts().WorkingDir.clear();
- if (BuildInvocation.getCodeGenOpts().DwarfVersion) {
- // It is necessary to explicitly set the DebugCompilationDir
- // to a common directory (e.g. root) if IgnoreCWD is true.
- // When IgnoreCWD is true, the module's content should not
- // depend on the current working directory. However, if dwarf
- // information is needed (when CGOpts.DwarfVersion is
- // non-zero), then CGOpts.DebugCompilationDir must be
- // populated, because otherwise the current working directory
- // will be automatically embedded in the dwarf information in
- // the pcm, contradicting the assumption that it is safe to
- // ignore the CWD. Thus in such cases,
- // CGOpts.DebugCompilationDir is explicitly set to a common
- // directory.
- // FIXME: It is still excessive to create a copy of
- // CodeGenOpts for each module. Since we do not modify the
- // CodeGenOpts otherwise per module, the following code
- // ends up generating identical CodeGenOpts for each module
- // with DebugCompilationDir pointing to the root directory.
- // We can optimize this away by creating a _single_ copy of
- // CodeGenOpts whose DebugCompilationDir points to the root
- // directory and reuse it across modules.
- BuildInvocation.getMutCodeGenOpts().DebugCompilationDir =
- llvm::sys::path::root_path(CWD);
- }
+ // To avoid clang inferring working directory from CWD, set to ignore
+ // compilation directory.
+ BuildInvocation.getMutCodeGenOpts().NoCompilationDir = true;
}
static std::vector<std::string> splitString(std::string S, char Separator) {
@@ -222,6 +201,9 @@ void dependencies::resetBenignCodeGenOptions(frontend::ActionKind ProgramAction,
CGOpts.ProfileInstrumentUsePath.clear();
CGOpts.SampleProfileFile.clear();
CGOpts.ProfileRemappingFile.clear();
+ // To avoid clang inferring compilation directory from CWD, set
+ // -no-compilation-dir option.
+ CGOpts.NoCompilationDir = true;
}
}
diff --git a/clang/test/ClangScanDeps/modules-debug-dir.c b/clang/test/ClangScanDeps/modules-debug-dir.c
index c4fb4982ed791..066f076e94524 100644
--- a/clang/test/ClangScanDeps/modules-debug-dir.c
+++ b/clang/test/ClangScanDeps/modules-debug-dir.c
@@ -7,6 +7,12 @@
// RUN: experimental-full -optimize-args=all > %t/result.json
// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s
+// RUN: %deps-to-rsp %t/result.json --module-name=mod > %t/mod.rsp
+// RUN: %clang @%t/mod.rsp -o %t/mod.pcm
+// RUN: llvm-dwarfdump --debug-info %t/mod.pcm | FileCheck %s --check-prefix=DWARF
+// DWARF: DW_TAG_compile_unit
+// DWARF-NOT: DW_AT_comp_dir
+
//--- cdb.json.in
[{
"directory": "DIR",
@@ -28,5 +34,6 @@ module mod {
// directory when current working directory optimization is in effect.
// CHECK: "modules": [
// CHECK: "command-line": [
-// CHECK: "-fdebug-compilation-dir={{\/|.*:(\\)?}}",
+// CHECK: "-fno-compilation-dir"
+// CHECK-NOT: -fdebug-compilation-dir
// CHECK: "translation-units": [
diff --git a/clang/test/CodeGen/debug-info-compilation-dir.c b/clang/test/CodeGen/debug-info-compilation-dir.c
index b49a0f5751f8e..195074764aa39 100644
--- a/clang/test/CodeGen/debug-info-compilation-dir.c
+++ b/clang/test/CodeGen/debug-info-compilation-dir.c
@@ -7,3 +7,10 @@
// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck -check-prefix=CHECK-DIR %s
// CHECK-DIR: CodeGen
+/// Test path remapping.
+// RUN: %clang_cc1 -fdebug-compilation-dir %S -main-file-name %s -emit-llvm -debug-info-kind=limited %s -o - | FileCheck -check-prefix=CHECK-ABS %s -DPREFIX=%S
+// CHECK-ABS: DIFile(filename: "[[PREFIX]]/debug-info-compilation-dir.c", directory: "[[PREFIX]]")
+
+// RUN: %clang_cc1 -fno-compilation-dir -main-file-name %s -emit-llvm -debug-info-kind=limited %s -o - | FileCheck -check-prefix=CHECK-NOMAP %s -DPREFIX=%S
+// CHECK-NOMAP: DIFile(filename: "[[PREFIX]]/debug-info-compilation-dir.c", directory: "")
+
|
Created using spr 1.3.6
| } | ||
| // To avoid clang inferring working directory from CWD, set to ignore | ||
| // compilation directory. | ||
| BuildInvocation.getMutCodeGenOpts().NoCompilationDir = true; |
There was a problem hiding this comment.
Yes I believe this change still make sure that the pcms contains no working directory on the command line if CWD optimization is on. The change looks safe to me.
Created using spr 1.3.6
|
I think at least with Bazel we use |
|
I am not planning to merge yet until debug info people approves the direction. Like you said, I considered The reason I want to touch this code:
|
|
Here is idea to not to add a new option. I am going to remove the fallback in CodeGen but make sure clang-driver does the fallback computation and clang cc1 will always respect that decision from clang-driver. In that case, it is a behavior change for cc1 flag, but not for a driver flag. |
& then for now, in your use case, you'd be using the cc1 flag via -Xclang, etc? I think that changing the driver to have the smarts there, but having cc1/frontend just respect whatever it's given sounds good to me, if a little subtle for now - but nothing drastic. |
Our explicit module build comes from dependency scanner, where it can generate the cc1 arguments we want to achieve empty compilation directory if needed. |
|
@dwblaikie The other possible implementation is here: #150112 It is so different and I don't feel it is really better so I used a separate PR. I guess the reason I feel it is not better because the entire system has lots of holes in it that just doesn't work in some cases regardless if how it is implemented, e.g.:
|
I worry that adding more distinct options is likely to complicate things further, though...
Are these existing bugs, or bugs that occur with the alternative patch? |
There are existing bugs so I am not worried about going either direction. I originally overdid the patch trying address them but then realize it should probably be saved for later. I am actually fine with either directions now. @adrian-prantl Do you have any preference. For the existing bug, I think it can be fixed by:
|
When enabling explicit gmodule builds, it is hard to enable unused
current working directory optimization because CWD is embedded in the
debug info when dependency scanner thinks the option is not needed.
This is true even when you pass in an empty
-fdebug-compiation-dirsince codegen will directly ask file system for CWD as a fallback,
causing different gmodules to be compiled in different directories.
Add an new cc1 flag
-fno-compilation-dirthat can makeDebugCompilationDir to be empty. This allows explicit module build to be
truely free from CWD dependencies when it needs the function. That can
reduce the number of clang modules to be generated in complex project
with a slight overhead for string deduplication in debug info.