HaohaiWen added inline comments.
================
Comment at: clang/include/clang/Driver/Options.td:1167
// GCC style -dumpdir. We intentionally don't implement the less useful
-dumpbase{,-ext}.
-def dumpdir : Separate<["-"], "dumpdir">, Flags<[CC1Option]>,
+def dumpdir : Separate<["-"], "dumpdir">, Flags<[CC1Option, CoreOption]>,
MetaVarName<"<dumppfx>">,
----------------
MaskRay wrote:
> This exposes `-dumpdir ` to clang-cl which may not be useful.
clang -gdwarf -gsplit-dwarf foo.cpp -o foo.exe -### will be expanded to
clang -cc1 ... "-dumpdir" "foo.exe-" .... "-split-dwarf-file" "foo.exe-foo.dwo"
dwo name will be prefixed by dumpdir if it's specified.
This dumpdir is required for clang-cl to have same dwo filename behavior as
clang.
================
Comment at: clang/lib/Driver/Driver.cpp:3898
if (!Args.hasArg(options::OPT_dumpdir)) {
+ Arg *FinalOutput = Args.getLastArg(options::OPT_o,
options::OPT__SLASH_o);
Arg *Arg = Args.MakeSeparateArg(
----------------
MaskRay wrote:
> Add a test to show how OPT__SLASH_o passes a `-dumpdir `to CC1.
Already tested in (-o is OPT__SLASH_o in cl mode)
```
clang/test/Driver/split-debug.c: 62
// RUN: %clang_cl -### --target=x86_64-unknown-windows-msvc -gsplit-dwarf -g %s
-o obj/out 2>&1 | FileCheck %s --check-prefix=SPLIT_LINK
/ SPLIT_LINK: "-dumpdir" "obj/out-"
// SPLIT_LINK: "-debug-info-kind=constructor"
// SPLIT_LINK-SAME: "-split-dwarf-file" "obj/out-split-debug.dwo"
"-split-dwarf-output" "obj/out-split-debug.dwo"
```
================
Comment at: clang/test/Driver/split-debug.c:19
// RUN: %clang -### -c -target amdgcn-amd-amdhsa -gsplit-dwarf -g %s 2>&1 |
FileCheck %s --check-prefix=SPLIT
+// RUN: %clang_cl -### -c -target x86_64-unknown-windows-msvc -gsplit-dwarf -g
%s 2>&1 | FileCheck %s --check-prefix=SPLIT
----------------
MaskRay wrote:
> MaskRay wrote:
> > Use `--target=` for new tests. `-target ` has been deprecated since Clang
> > 3.x
> You can use `-gno-split-dwarf -gsplit-dwarf` to test `CoreOption` on
> `-gno-split-dwarf`.
Good suggestion.
================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:1895
+ if (RelocatedSection != Obj.section_end() && Name.contains(".dwo")) {
+ // Each section in COFF can directly contain relocations.
+ if (isa<COFFObjectFile>(&Obj) && Section.relocations().empty())
----------------
MaskRay wrote:
> skan wrote:
> > The comment is confusing according to line 1900. Could you refine it?
> I'll change this to `ends_with(".dwo")` separately.
>
> `check-llvm check-clang` passes even if I remove `if
> (isa<COFFObjectFile>(&Obj) && Section.relocations().empty()) continue`. What
> does it do?
For ELF, if section a.dwo has relocations, it'll have a a.dwo.rela section,
this section is RelocatedSection.
All dwo section should not have relocations, so if this is a dwo file, then
RelocatedSection should be Obj.section_end(). Otherwise it will report warning.
For COFF, relocations of section A are directly stored in section A.
RelocatedSection is Section itself so it will never be Obj.section_end(). Then
this warning will always be reported. What we need to do is to check
Section.relocations().empty() for COFF sections.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152785/new/
https://reviews.llvm.org/D152785
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
