[PATCH] D85685: Support dwarf fission for wasm object files

2020-09-17 Thread Derek Schuff via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0ff28fa6a756: Support dwarf fission for wasm object files 
(authored by dschuff).

Changed prior to commit:
  https://reviews.llvm.org/D85685?vs=292624&id=292626#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/split-debug.c
  llvm/include/llvm/MC/MCWasmObjectWriter.h
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/lib/MC/MCAsmBackend.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/test/DebugInfo/WebAssembly/fission-cu.ll
  llvm/test/DebugInfo/WebAssembly/fission-sections.ll

Index: llvm/test/DebugInfo/WebAssembly/fission-sections.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/fission-sections.ll
@@ -0,0 +1,48 @@
+; RUN: llc -split-dwarf-file=baz.dwo -split-dwarf-output=%t.dwo  -O0 %s -mtriple=wasm32-unknown-unknown -filetype=obj -o %t
+; RUN: llvm-objdump -h %t | FileCheck --check-prefix=OBJ %s
+; RUN: llvm-objdump -h %t.dwo | FileCheck --check-prefix=DWO %s
+
+
+; This test is derived from test/DebugInfo/X86/fission-cu.ll
+; But it checks that the output objects have the expected sections
+
+source_filename = "test/DebugInfo/WebAssembly/fission-cu.ll"
+
+@a = global i32 0, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!4}
+!llvm.module.flags = !{!7}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = !DIGlobalVariable(name: "a", scope: null, file: !2, line: 1, type: !3, isLocal: false, isDefinition: true)
+!2 = !DIFile(filename: "baz.c", directory: "/usr/local/google/home/echristo/tmp")
+!3 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!4 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang version 3.3 (trunk 169021) (llvm/trunk 169020)", isOptimized: false, runtimeVersion: 0, splitDebugFilename: "baz.dwo", emissionKind: FullDebug, enums: !5, retainedTypes: !5, globals: !6, imports: !5)
+!5 = !{}
+!6 = !{!0}
+!7 = !{i32 1, !"Debug Info Version", i32 3}
+
+; CHECK-LABEL: Sections:
+
+; OBJ: Idx Name
+; OBJ-NEXT: 0 IMPORT
+; OBJ-NEXT: DATACOUNT
+; OBJ-NEXT: DATA
+; OBJ-NEXT: .debug_abbrev
+; OBJ-NEXT: .debug_info
+; OBJ-NEXT: .debug_str
+; OBJ-NEXT: .debug_addr
+; OBJ-NEXT: .debug_pubnames
+; OBJ-NEXT: .debug_pubtypes
+; OBJ-NEXT: .debug_line
+; OBJ-NEXT: linking
+
+
+; DWO: Idx Name
+; DWO-NOT: IMPORT
+; DWO-NOT: DATA
+; DWO: 0 .debug_str.dwo
+; DWO-NEXT: .debug_str_offsets.dwo
+; DWO-NEXT: .debug_info.dwo
+; DWO-NEXT: .debug_abbrev.dwo
+; DWO-NEXT: producers
Index: llvm/test/DebugInfo/WebAssembly/fission-cu.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/fission-cu.ll
@@ -0,0 +1,121 @@
+; RUN: llc -split-dwarf-file=baz.dwo  -O0 %s -mtriple=wasm32-unknown-unknown -filetype=obj -o %t
+; RUN: llvm-dwarfdump -v -all %t | FileCheck %s
+; RUN: llvm-readobj --relocations %t | FileCheck --check-prefix=OBJ %s
+; RUN: llvm-objdump -h %t | FileCheck --check-prefix=HDR %s
+
+; This test is derived from test/DebugInfo/X86/fission-cu.ll
+
+source_filename = "test/DebugInfo/WebAssembly/fission-cu.ll"
+
+@a = global i32 0, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!4}
+!llvm.module.flags = !{!7}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = !DIGlobalVariable(name: "a", scope: null, file: !2, line: 1, type: !3, isLocal: false, isDefinition: true)
+!2 = !DIFile(filename: "baz.c", directory: "/usr/local/google/home/echristo/tmp")
+!3 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!4 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang version 3.3 (trunk 169021) (llvm/trunk 169020)", isOptimized: false, runtimeVersion: 0, splitDebugFilename: "baz.dwo", emissionKind: FullDebug, enums: !5, retainedTypes: !5, globals: !6, imports: !5)
+!5 = !{}
+; Check that the skeleton compile unit contains the proper attributes:
+; This DIE has the following attributes: DW_AT_comp_dir, DW_AT_stmt_list,
+; DW_AT_low_pc, DW_AT_high_pc, DW_AT_ranges, DW_AT_dwo_name, DW_AT_dwo_id,
+; DW_AT_ranges_base, DW_AT_addr_base.
+
+; CHECK: .debug_abbrev contents:
+; CHECK: Abbrev table for offset: 0x
+; CHECK: [1] DW_TAG_compile_unit DW_CHILDREN_no
+; CHECK: DW_AT_stmt_list DW_FORM_sec_offset
+; CHECK: DW_AT_comp_dir  DW_FORM_strp
+; CHECK: DW_AT_GNU_dwo_name  DW_FORM_strp
+; CHECK: DW_AT_GNU_dwo_idDW_FORM_data8
+
+; Check that we're using the right forms.
+; CHECK: .debug_abbrev.dwo contents:
+; CHECK: Abbrev table for offset: 0x
+; CHECK: [1] DW_TAG_compile_unit DW_CHILDREN_yes
+; CHECK: DW_AT_producer  DW_FORM_GNU_str_index
+; CHECK: DW_AT_language  DW_FORM_data2
+; CHECK: DW_AT_name  DW_FORM

[PATCH] D85685: Support dwarf fission for wasm object files

2020-09-17 Thread Derek Schuff via Phabricator via cfe-commits 
dschuff updated this revision to Diff 292624.
dschuff added a comment.

rebase, address comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

Files:
  llvm/lib/MC/WasmObjectWriter.cpp


Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -1309,8 +1309,9 @@
 support::endian::Writer DwoWriter(*DwoOS, support::little);
 W = &DwoWriter;
 return TotalSize + writeOneObject(Asm, Layout, DwoMode::DwoOnly);
+  } else {
+return writeOneObject(Asm, Layout, DwoMode::AllSections);
   }
-  return writeOneObject(Asm, Layout, DwoMode::AllSections);
 }
 
 uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm,


Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -1309,8 +1309,9 @@
 support::endian::Writer DwoWriter(*DwoOS, support::little);
 W = &DwoWriter;
 return TotalSize + writeOneObject(Asm, Layout, DwoMode::DwoOnly);
+  } else {
+return writeOneObject(Asm, Layout, DwoMode::AllSections);
   }
-  return writeOneObject(Asm, Layout, DwoMode::AllSections);
 }
 
 uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85685: Support dwarf fission for wasm object files

2020-09-17 Thread Wouter van Oortmerssen via Phabricator via cfe-commits 
aardappel added inline comments.



Comment at: llvm/lib/MC/WasmObjectWriter.cpp:1313
+  }
+  return writeOneObject(Asm, Layout, DwoMode::AllSections);
+}

Nit picking on this because I can't find anything else to complain about:
This line would be more readable inside an `else`.
I know LLVM prefers "early out" (and so do I), but here IsSplitDwarf is really 
not an early out case since it is the most complex case, and !IsSplitDwarf is 
not great for early out either because it is the common case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

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

[PATCH] D85685: Support dwarf fission for wasm object files

2020-09-16 Thread Sam Clegg via Phabricator via cfe-commits 
sbc100 accepted this revision.
sbc100 added a comment.

I don't really grok the `TargetFrameLowering::DwarfFrameBase` part but 
everything else LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

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

[PATCH] D85685: Support dwarf fission for wasm object files

2020-09-16 Thread Derek Schuff via Phabricator via cfe-commits 
dschuff added a comment.

In D85685#2277998 , @dschuff wrote:

> 



> Yeah, ELF splits ELFWriter out from ELFObjectWriter, and then instantiates it 
> twice. It's all because ELFObjectWriter has to derive from MCObjectWriter 
> which was clearly not designed with this in mind. I found the class split to 
> be a bit awkward, but I don't really have strong feelings about it either way.

I should add that what I do instead is just have one instance, and just reset 
the relevant state before calling writeOneObject again. So the structure is 
cleaner but the downside of that is that the state has to be manually reset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

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

[PATCH] D85685: Support dwarf fission for wasm object files

2020-09-16 Thread Derek Schuff via Phabricator via cfe-commits 
dschuff added a comment.

In D85685#2275585 , @sbc100 wrote:

> Seems reasonable.   Do you think this way is cleaner than the way elf does 
> it?   Looks like ELF creates two different ELFWriter inside the 
> ELFDwoObjectWriter subclass right?

Yeah, ELF splits ELFWriter out from ELFObjectWriter, and then instantiates it 
twice. It's all because ELFObjectWriter has to derive from MCObjectWriter which 
was clearly not designed with this in mind. I found the class split to be a bit 
awkward, but I don't really have strong feelings about it either way.

> Are we going need to wasm-ld tests to followup or is this really independent 
> of the linker?

It should be independent of the linker because the dwo files don't get linked 
by the linker. They can be used independently (or combined by the `dwp` tool 
but AFAIK it's simpler than the linker). And the object files are just the same 
as usual from the linker's perspective.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

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

[PATCH] D85685: Support dwarf fission for wasm object files

2020-09-15 Thread Sam Clegg via Phabricator via cfe-commits 
sbc100 added a comment.

Seems reasonable.   Do you think this way is cleaner than the way elf does it?  
 Looks like ELF creates two different ELFWriter inside the ELFDwoObjectWriter 
subclass right?

Are we going need to wasm-ld tests to followup or is this really independent of 
the linker?




Comment at: llvm/lib/MC/WasmObjectWriter.cpp:224
 class WasmObjectWriter : public MCObjectWriter {
-  support::endian::Writer W;
+  support::endian::Writer *W;
 

Kind of a same this change means changing so many line in this file ..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

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

[PATCH] D85685: Support dwarf fission for wasm object files

2020-09-15 Thread Derek Schuff via Phabricator via cfe-commits 
dschuff added a comment.

OK, I think this is ready to review for real. Can you take another look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

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