[PATCH] D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN if -fthinlto-index= is specified

2021-01-13 Thread David Blaikie via Phabricator via cfe-commits
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks alright. I wouldn't mind knowing a bit more about how these things 
interact if different types of input files are listed together in a single 
command line, though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94647/new/

https://reviews.llvm.org/D94647

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN if -fthinlto-index= is specified

2021-01-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D94647#2497170 , @dblaikie wrote:

> In implicit thinlto this seemed to work for me without any changes:
>
>   $ clang++-tot -flto=thin test.cpp -g -c
>   $ clang++-tot -flto=thin -fuse-ld=lld -gsplit-dwarf test.o && 
> llvm-dwarfdump-tot a.out
>   a.out:  file format elf64-x86-64
>   
>   .debug_info contents:
>   0x: Compile Unit: length = 0x002c, format = DWARF32, version = 
> 0x0004, abbr_offset = 0x, addr_size = 0x08 (next unit at 0x0030)
>   
>   0x000b: DW_TAG_compile_unit
> DW_AT_stmt_list   (0x)
> DW_AT_comp_dir
> ("/usr/local/google/home/blaikie/dev/scratch")
> DW_AT_GNU_dwo_name("a.out_dwo/1.dwo")
> DW_AT_GNU_dwo_id  (0xf29563d7c812deae)
> DW_AT_low_pc  (0x00201730)
> DW_AT_high_pc (0x00201738)
> DW_AT_GNU_addr_base   (0x)
>
> Any idea why that worked without the need for a special case/changes? Might 
> be nice if both these use cases (implicit and explicit thinlto) somehow 
> managed to use more common code generation options. But I guess that's too 
> different - probably some generic "take all the clang flags and pass them 
> through to the linker to pass back to the compiler", which isn't really 
> relevant to this... well, maybe (if they're being passed back to the compiler 
> in implicit thin lto, how does implicit thin lto work correctly without the 
> -g flag?)

Interesting. D44788  let Clang driver pass 
`-plugin-opt=dwo_dir=` to the linker. LTOBackend.cpp emits 
`$dwo_dir/[1234].dwo`.

There are .dwo files but the file names are a bit less ideal. For distributed 
ThinLTO, I think specifying the dwo filename explicitly is better.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94647/new/

https://reviews.llvm.org/D94647

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN if -fthinlto-index= is specified

2021-01-13 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In implicit thinlto this seemed to work for me without any changes:

  $ clang++-tot -flto=thin test.cpp -g -c
  $ clang++-tot -flto=thin -fuse-ld=lld -gsplit-dwarf test.o && 
llvm-dwarfdump-tot a.out
  a.out:  file format elf64-x86-64
  
  .debug_info contents:
  0x: Compile Unit: length = 0x002c, format = DWARF32, version = 
0x0004, abbr_offset = 0x, addr_size = 0x08 (next unit at 0x0030)
  
  0x000b: DW_TAG_compile_unit
DW_AT_stmt_list   (0x)
DW_AT_comp_dir("/usr/local/google/home/blaikie/dev/scratch")
DW_AT_GNU_dwo_name("a.out_dwo/1.dwo")
DW_AT_GNU_dwo_id  (0xf29563d7c812deae)
DW_AT_low_pc  (0x00201730)
DW_AT_high_pc (0x00201738)
DW_AT_GNU_addr_base   (0x)

Any idea why that worked without the need for a special case/changes? Might be 
nice if both these use cases (implicit and explicit thinlto) somehow managed to 
use more common code generation options. But I guess that's too different - 
probably some generic "take all the clang flags and pass them through to the 
linker to pass back to the compiler", which isn't really relevant to this... 
well, maybe (if they're being passed back to the compiler in implicit thin lto, 
how does implicit thin lto work correctly without the -g flag?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94647/new/

https://reviews.llvm.org/D94647

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN if -fthinlto-index= is specified

2021-01-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 316558.
MaskRay added a comment.

improve code comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94647/new/

https://reviews.llvm.org/D94647

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/split-debug.c


Index: clang/test/Driver/split-debug.c
===
--- clang/test/Driver/split-debug.c
+++ clang/test/Driver/split-debug.c
@@ -24,6 +24,14 @@
 /// -gsplit-dwarf is a no-op if no -g is specified.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf %s 2>&1 | FileCheck %s 
--check-prefix=G0
 
+/// ... unless -fthinlto-index= is specified.
+// RUN: echo > %t.bc
+// RUN: %clang -### -c -target x86_64 -fthinlto-index=dummy -gsplit-dwarf 
%t.bc 2>&1 | FileCheck %s --check-prefix=THINLTO
+
+// THINLTO-NOT:  "-debug-info-kind=
+// THINLTO:  "-ggnu-pubnames"
+// THINLTO-SAME: "-split-dwarf-file" "{{.*}}.dwo" "-split-dwarf-output" 
"{{.*}}.dwo"
+
 /// -gno-split-dwarf disables debug fission.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf -g -gno-split-dwarf %s 
2>&1 | FileCheck %s --check-prefix=NOSPLIT
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=single -g -gno-split-dwarf 
%s 2>&1 | FileCheck %s --check-prefix=NOSPLIT
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3754,7 +3754,12 @@
   Args.hasFlag(options::OPT_fsplit_dwarf_inlining,
options::OPT_fno_split_dwarf_inlining, false);
 
-  if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
+  // Normally -gsplit-dwarf is only useful with -g. For -gsplit-dwarf in 
ThinLTO
+  // backend phase which does object file generation and no IR generation, -g
+  // should not be needed. So allow -gsplit-dwarf with either -g or
+  // -fthinlto-index=.
+  if (Args.hasArg(options::OPT_g_Group) ||
+  Args.hasArg(options::OPT_fthinlto_index_EQ)) {
 Arg *SplitDWARFArg;
 DwarfFission = getDebugFissionKind(D, Args, SplitDWARFArg);
 if (DwarfFission != DwarfFissionKind::None &&
@@ -3762,7 +3767,8 @@
   DwarfFission = DwarfFissionKind::None;
   SplitDWARFInlining = false;
 }
-
+  }
+  if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
 DebugInfoKind = codegenoptions::LimitedDebugInfo;
 
 // If the last option explicitly specified a debug-info level, use it.


Index: clang/test/Driver/split-debug.c
===
--- clang/test/Driver/split-debug.c
+++ clang/test/Driver/split-debug.c
@@ -24,6 +24,14 @@
 /// -gsplit-dwarf is a no-op if no -g is specified.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf %s 2>&1 | FileCheck %s --check-prefix=G0
 
+/// ... unless -fthinlto-index= is specified.
+// RUN: echo > %t.bc
+// RUN: %clang -### -c -target x86_64 -fthinlto-index=dummy -gsplit-dwarf %t.bc 2>&1 | FileCheck %s --check-prefix=THINLTO
+
+// THINLTO-NOT:  "-debug-info-kind=
+// THINLTO:  "-ggnu-pubnames"
+// THINLTO-SAME: "-split-dwarf-file" "{{.*}}.dwo" "-split-dwarf-output" "{{.*}}.dwo"
+
 /// -gno-split-dwarf disables debug fission.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf -g -gno-split-dwarf %s 2>&1 | FileCheck %s --check-prefix=NOSPLIT
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=single -g -gno-split-dwarf %s 2>&1 | FileCheck %s --check-prefix=NOSPLIT
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3754,7 +3754,12 @@
   Args.hasFlag(options::OPT_fsplit_dwarf_inlining,
options::OPT_fno_split_dwarf_inlining, false);
 
-  if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
+  // Normally -gsplit-dwarf is only useful with -g. For -gsplit-dwarf in ThinLTO
+  // backend phase which does object file generation and no IR generation, -g
+  // should not be needed. So allow -gsplit-dwarf with either -g or
+  // -fthinlto-index=.
+  if (Args.hasArg(options::OPT_g_Group) ||
+  Args.hasArg(options::OPT_fthinlto_index_EQ)) {
 Arg *SplitDWARFArg;
 DwarfFission = getDebugFissionKind(D, Args, SplitDWARFArg);
 if (DwarfFission != DwarfFissionKind::None &&
@@ -3762,7 +3767,8 @@
   DwarfFission = DwarfFissionKind::None;
   SplitDWARFInlining = false;
 }
-
+  }
+  if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
 DebugInfoKind = codegenoptions::LimitedDebugInfo;
 
 // If the last option explicitly specified a debug-info level, use it.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN if -fthinlto-index= is specified

2021-01-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision.
MaskRay added reviewers: dblaikie, inglorion, thakis.
Herald added a subscriber: arphaman.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

-g is an IR generation option while -gsplit-dwarf is an object file generation 
option.
-gsplit-dwarf should not be needed for ThinLTO backend compile, which does 
object file generation.

`-fthinlto-index= -gsplit-dwarf` emits .dwo even in the absence of -g.
This should fix https://crbug.com/1158215

  // Distributed ThinLTO usage
  clang -g -O2 -c -flto=thin -fthin-link-bitcode=a.indexing.o a.c
  clang -g -O2 -c -flto=thin -fthin-link-bitcode=b.indexing.o b.c
  clang -fuse-ld=lld -Wl,--thinlto-index-only=a.rsp 
-Wl,--thinlto-prefix-replace=';lto/' 
-Wl,--thinlto-object-suffix-replace='.indexing.o;.o' a.indexing.o b.indexing.o
  clang -gsplit-dwarf -O2 -c -fthinlto-index=lto/a.o.thinlto.bc a.o -o lto/a.o
  clang -gsplit-dwarf -O2 -c -fthinlto-index=lto/b.o.thinlto.bc b.o -o lto/b.o
  clang -fuse-ld=lld @a.rsp -o exe


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94647

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/split-debug.c


Index: clang/test/Driver/split-debug.c
===
--- clang/test/Driver/split-debug.c
+++ clang/test/Driver/split-debug.c
@@ -24,6 +24,14 @@
 /// -gsplit-dwarf is a no-op if no -g is specified.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf %s 2>&1 | FileCheck %s 
--check-prefix=G0
 
+/// ... unless -fthinlto-index= is specified.
+// RUN: echo > %t.bc
+// RUN: %clang -### -c -target x86_64 -fthinlto-index=dummy -gsplit-dwarf 
%t.bc 2>&1 | FileCheck %s --check-prefix=THINLTO
+
+// THINLTO-NOT:  "-debug-info-kind=
+// THINLTO:  "-ggnu-pubnames"
+// THINLTO-SAME: "-split-dwarf-file" "{{.*}}.dwo" "-split-dwarf-output" 
"{{.*}}.dwo"
+
 /// -gno-split-dwarf disables debug fission.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf -g -gno-split-dwarf %s 
2>&1 | FileCheck %s --check-prefix=NOSPLIT
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=single -g -gno-split-dwarf 
%s 2>&1 | FileCheck %s --check-prefix=NOSPLIT
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3754,7 +3754,11 @@
   Args.hasFlag(options::OPT_fsplit_dwarf_inlining,
options::OPT_fno_split_dwarf_inlining, false);
 
-  if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
+  // Normally -gsplit-dwarf is only useful with -g. -fthinlto-index= reads in
+  // bitcode files (which may be compiled with -g) and emits an object file.
+  // Allow -gsplit-dwarf with -fthinlto-index= as well.
+  if (Args.hasArg(options::OPT_g_Group) ||
+  Args.hasArg(options::OPT_fthinlto_index_EQ)) {
 Arg *SplitDWARFArg;
 DwarfFission = getDebugFissionKind(D, Args, SplitDWARFArg);
 if (DwarfFission != DwarfFissionKind::None &&
@@ -3762,7 +3766,8 @@
   DwarfFission = DwarfFissionKind::None;
   SplitDWARFInlining = false;
 }
-
+  }
+  if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
 DebugInfoKind = codegenoptions::LimitedDebugInfo;
 
 // If the last option explicitly specified a debug-info level, use it.


Index: clang/test/Driver/split-debug.c
===
--- clang/test/Driver/split-debug.c
+++ clang/test/Driver/split-debug.c
@@ -24,6 +24,14 @@
 /// -gsplit-dwarf is a no-op if no -g is specified.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf %s 2>&1 | FileCheck %s --check-prefix=G0
 
+/// ... unless -fthinlto-index= is specified.
+// RUN: echo > %t.bc
+// RUN: %clang -### -c -target x86_64 -fthinlto-index=dummy -gsplit-dwarf %t.bc 2>&1 | FileCheck %s --check-prefix=THINLTO
+
+// THINLTO-NOT:  "-debug-info-kind=
+// THINLTO:  "-ggnu-pubnames"
+// THINLTO-SAME: "-split-dwarf-file" "{{.*}}.dwo" "-split-dwarf-output" "{{.*}}.dwo"
+
 /// -gno-split-dwarf disables debug fission.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf -g -gno-split-dwarf %s 2>&1 | FileCheck %s --check-prefix=NOSPLIT
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=single -g -gno-split-dwarf %s 2>&1 | FileCheck %s --check-prefix=NOSPLIT
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3754,7 +3754,11 @@
   Args.hasFlag(options::OPT_fsplit_dwarf_inlining,
options::OPT_fno_split_dwarf_inlining, false);
 
-  if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
+  // Normally -gsplit-dwarf is only useful with -g. -fthinlto-index= reads in
+  // bitcode files (which may be compiled with -g) and emits an object file.
+  // Allow -gsplit-dwarf with