[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D88603#2361010 , @dblaikie wrote:

> In D88603#2360845 , @dschuff wrote:
>
>> @sbc100 I found that the cause of the assertion is that 
>> in dwarf 5, the type units apparently go in the .debug_info section (instead 
>> of the .debug_type section), and this section already exists (but it was 
>> created as a non-comdat).
>> So when `getWasmSection` tries to look up an existing section it fails 
>> because the group is part of the key. Then it tries to create a new section 
>> but that fails because the name is duplicate.
>>
>> For some reason this doesn't happen or is not a problem with ELF but I 
>> haven't looked up why yet.
>> For now I just disabled the dwarf5+TU tests since we don't really use dwarf5 
>> yet anyway.
>
> for ELF, each {section name, hash} pair denotes a distinct section (when 
> using SHF_GROUP). Linker's deduplicate the group based on the hash, then 
> squish all the distinct sections with the same name together. (same system 
> that powers -ffunction-sections -fno-unique-section-names - even though each 
> function section is called ".text", because they have different hashes and 
> use SHF_GROUP, they are distinct sections in the object file)

Without similar behavior for wasm, how does wasm deduplicate these type units, 
if they're all in one section together? (have you tested this with two types? 
I'd expect that to hit the same problem - of attempting to create a section 
that already exists when it went to create the second type unit)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D88603#2360845 , @dschuff wrote:

> @sbc100 I found that the cause of the assertion is that 
> in dwarf 5, the type units apparently go in the .debug_info section (instead 
> of the .debug_type section), and this section already exists (but it was 
> created as a non-comdat).
> So when `getWasmSection` tries to look up an existing section it fails 
> because the group is part of the key. Then it tries to create a new section 
> but that fails because the name is duplicate.
>
> For some reason this doesn't happen or is not a problem with ELF but I 
> haven't looked up why yet.
> For now I just disabled the dwarf5+TU tests since we don't really use dwarf5 
> yet anyway.

for ELF, each {section name, hash} pair denotes a distinct section (when using 
SHF_GROUP). Linker's deduplicate the group based on the hash, then squish all 
the distinct sections with the same name together. (same system that powers 
-ffunction-sections -fno-unique-section-names - even though each function 
section is called ".text", because they have different hashes and use 
SHF_GROUP, they are distinct sections in the object file)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

@sbc100 I found that the cause of the assertion is that 
in dwarf 5, the type units apparently go in the .debug_info section (instead of 
the .debug_type section), and this section already exists (but it was created 
as a non-comdat).
So when `getWasmSection` tries to look up an existing section it fails because 
the group is part of the key. Then it tries to create a new section but that 
fails because the name is duplicate.

For some reason this doesn't happen or is not a problem with ELF but I haven't 
looked up why yet.
For now I just disabled the dwarf5+TU tests since we don't really use dwarf5 
yet anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff updated this revision to Diff 301490.
dschuff added a comment.

fix diff; it should be against master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll

Index: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
@@ -0,0 +1,117 @@
+; RUN: llc -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-4
+
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN: -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-4
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-4
+
+; TODO: enable testing for dwarf v5 with type units
+; (See the FIXME in MCObjectFileInfo::getDwarfComdatSection)
+; RU N: llc -dwarf-version=5 -generate-type-units \
+; RU N: -filetype=obj -O0 -mtriple= < %s \
+; RU N: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-5
+
+; RU N: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RU N: -dwarf-version=5 -generate-type-units \
+; RU N: -filetype=obj -O0 -mtriple= < %s \
+; RU N: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-5
+; RU N: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-5
+
+; This test is derived from test/CodeGen/X86/dwarf-headers.ll
+
+; Looking for DWARF headers to be generated correctly.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)
+
+; Test case built from:
+;struct S {
+;  int s1;
+;};
+;
+;S s;
+
+; Verify the v4 non-split headers.
+; Note that we check the exact offset of the DIEs because that tells us
+; the length of the header.
+;
+; SINGLE-4: .debug_info contents:
+; SINGLE-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x000b: DW_TAG_compile_unit
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v4 split headers.
+;
+; O-4: .debug_info contents:
+; O-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; O-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_info.dwo contents:
+; DWO-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_types.dwo contents:
+; DWO-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v5 non-split headers. Type units come first.
+; All .debug_info sections are reported in one go, but the offset resets for
+; each new section.
+;
+; SINGLE-5: .debug_info contents:
+; SINGLE-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_type, abbr_offset
+; SINGLE-5: 0x0018: DW_TAG_type_unit
+; SINGLE-5-NOT: contents:
+; SINGLE-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_compile, abbr_offset
+; SINGLE-5: 0x000c: DW_TAG_compile_unit
+
+; Verify the v5 split headers.
+;
+; O-5: .debug_info contents:
+; O-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset
+; O-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; O-5: 0x0014: DW_TAG_skeleton_unit 
+;
+; DWO-5: .debug_info.dwo contents:
+; DWO-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_type, abbr_offset
+; DWO-5: 0x0018: DW_TAG_type_unit
+; DWO-5: 0x0033: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_compile, abbr_offset
+; DWO-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; DWO-5: 0x0047: DW_TAG_compile_unit
+
+
+; ModuleID = 't.cpp'
+source_filename = "t.cpp"
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown-wasm"
+
+%struct.S = type { i32 }
+
+@s = global %struct.S zeroinitializer, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11}
+!llvm.ident = !{!12}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "s", scope: !2, file: !3, line: 5, type: !6, isLocal: false, isDefinition: true)
+!2 

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff updated this revision to Diff 301488.
dschuff added a comment.

can't just getOrCreate symbol without dealing with the section
just disable the test for now as we don't need dw5


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

Files:
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll

Index: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
@@ -0,0 +1,117 @@
+; RUN: llc -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-4
+
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN: -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-4
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-4
+
+; TODO: enable testing for dwarf v5 with type units
+; (See the FIXME in MCObjectFileInfo::getDwarfComdatSection)
+; RU N: llc -dwarf-version=5 -generate-type-units \
+; RU N: -filetype=obj -O0 -mtriple= < %s \
+; RU N: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-5
+
+; RU N: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RU N: -dwarf-version=5 -generate-type-units \
+; RU N: -filetype=obj -O0 -mtriple= < %s \
+; RU N: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-5
+; RU N: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-5
+
+; This test is derived from test/CodeGen/X86/dwarf-headers.ll
+
+; Looking for DWARF headers to be generated correctly.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)
+
+; Test case built from:
+;struct S {
+;  int s1;
+;};
+;
+;S s;
+
+; Verify the v4 non-split headers.
+; Note that we check the exact offset of the DIEs because that tells us
+; the length of the header.
+;
+; SINGLE-4: .debug_info contents:
+; SINGLE-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x000b: DW_TAG_compile_unit
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v4 split headers.
+;
+; O-4: .debug_info contents:
+; O-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; O-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_info.dwo contents:
+; DWO-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_types.dwo contents:
+; DWO-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v5 non-split headers. Type units come first.
+; All .debug_info sections are reported in one go, but the offset resets for
+; each new section.
+;
+; SINGLE-5: .debug_info contents:
+; SINGLE-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_type, abbr_offset
+; SINGLE-5: 0x0018: DW_TAG_type_unit
+; SINGLE-5-NOT: contents:
+; SINGLE-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_compile, abbr_offset
+; SINGLE-5: 0x000c: DW_TAG_compile_unit
+
+; Verify the v5 split headers.
+;
+; O-5: .debug_info contents:
+; O-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset
+; O-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; O-5: 0x0014: DW_TAG_skeleton_unit 
+;
+; DWO-5: .debug_info.dwo contents:
+; DWO-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_type, abbr_offset
+; DWO-5: 0x0018: DW_TAG_type_unit
+; DWO-5: 0x0033: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_compile, abbr_offset
+; DWO-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; DWO-5: 0x0047: DW_TAG_compile_unit
+
+
+; ModuleID = 't.cpp'
+source_filename = "t.cpp"
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown-wasm"
+
+%struct.S = type { i32 }
+
+@s = global %struct.S zeroinitializer, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11}
+!llvm.ident = !{!12}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "s", scope: !2, file: !3, line: 5, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: 

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff updated this revision to Diff 301486.
dschuff added a comment.

use getOrCreate


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/WasmObjectWriter.cpp


Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -1378,9 +1378,6 @@
   MCSymbol *Begin = Sec.getBeginSymbol();
   if (Begin) {
 WasmIndices[cast(Begin)] = CustomSections.size();
-if (SectionName != Begin->getName())
-  report_fatal_error("section name and begin symbol should match: " +
- Twine(SectionName));
   }
 
   // Separate out the producers and target features sections
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -963,9 +963,11 @@
   case Triple::ELF:
 return Ctx->getELFSection(Name, ELF::SHT_PROGBITS, ELF::SHF_GROUP, 0,
   utostr(Hash));
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   MCContext::GenericSectionID);
   case Triple::MachO:
   case Triple::COFF:
-  case Triple::Wasm:
   case Triple::GOFF:
   case Triple::XCOFF:
   case Triple::UnknownObjectFormat:
Index: llvm/lib/MC/MCContext.cpp
===
--- llvm/lib/MC/MCContext.cpp
+++ llvm/lib/MC/MCContext.cpp
@@ -644,7 +644,7 @@
 
   StringRef CachedName = Entry.first.SectionName;
 
-  MCSymbol *Begin = createSymbol(CachedName, false, false);
+  MCSymbol *Begin = getOrCreateSymbol(CachedName);
   cast(Begin)->setType(wasm::WASM_SYMBOL_TYPE_SECTION);
 
   MCSectionWasm *Result = new (WasmAllocator.Allocate())
Index: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -394,8 +394,9 @@
 UseSectionsAsReferences = DwarfSectionsAsReferences == Enable;
 
   // Don't generate type units for unsupported object file formats.
-  GenerateTypeUnits =
-  A->TM.getTargetTriple().isOSBinFormatELF() && GenerateDwarfTypeUnits;
+  GenerateTypeUnits = (A->TM.getTargetTriple().isOSBinFormatELF() ||
+   A->TM.getTargetTriple().isOSBinFormatWasm()) &&
+  GenerateDwarfTypeUnits;
 
   TheAccelTableKind = computeAccelTableKind(
   DwarfVersion, GenerateTypeUnits, DebuggerTuning, 
A->TM.getTargetTriple());
Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -214,6 +214,9 @@
 // RUN: %clang -### -fdebug-types-section -fno-debug-types-section -target 
x86_64-unknown-linux %s 2>&1 \
 // RUN:| FileCheck -check-prefix=NOFDTS %s
 //
+// RUN: %clang -### -fdebug-types-section -target wasm32-unknown-unknown %s 
2>&1 \
+// RUN:| FileCheck -check-prefix=FDTS %s
+//
 // RUN: %clang -### -fdebug-types-section -target x86_64-apple-darwin %s 2>&1 \
 // RUN:| FileCheck -check-prefix=FDTSE %s
 //
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3989,7 +3989,7 @@
 
   if (Args.hasFlag(options::OPT_fdebug_types_section,
options::OPT_fno_debug_types_section, false)) {
-if (!T.isOSBinFormatELF()) {
+if (!(T.isOSBinFormatELF() || T.isOSBinFormatWasm())) {
   D.Diag(diag::err_drv_unsupported_opt_for_target)
   << Args.getLastArg(options::OPT_fdebug_types_section)
  ->getAsString(Args)


Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -1378,9 +1378,6 @@
   MCSymbol *Begin = Sec.getBeginSymbol();
   if (Begin) {
 WasmIndices[cast(Begin)] = CustomSections.size();
-if (SectionName != Begin->getName())
-  report_fatal_error("section name and begin symbol should match: " +
- Twine(SectionName));
   }
 
   // Separate out the producers and target features sections
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -963,9 +963,11 @@
   case 

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

In D88603#2359554 , @dblaikie wrote:

> In D88603#2357973 , @dschuff wrote:
>
>> This broke the bots for some strange reason that didn't reproduce locally. 
>> But because it was ~all of them, I probably just did something stupid.
>
> Good to have links to buildbots and/or quotes from buildbots (incase the logs 
> get retired) about the specifics so other folks can chip in/know what went 
> wrong, etc.

Example link is http://lab.llvm.org:8011/#/builders/109/builds/1402
But, as I suspected, I just did something stupid (namely, I thought I had 
assertions enabled, but I didn't)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D88603#2357973 , @dschuff wrote:

> This broke the bots for some strange reason that didn't reproduce locally. 
> But because it was ~all of them, I probably just did something stupid.

Good to have links to buildbots and/or quotes from buildbots (incase the logs 
get retired) about the specifics so other folks can chip in/know what went 
wrong, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

This broke the bots for some strange reason that didn't reproduce locally. But 
because it was ~all of them, I probably just did something stupid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[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 rGbcb8a119df21: [WebAssembly] Add support for DWARF type units 
(authored by dschuff).

Changed prior to commit:
  https://reviews.llvm.org/D88603?vs=301133=301136#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll

Index: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
@@ -0,0 +1,116 @@
+; RUN: llc -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-4
+
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN: -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-4
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-4
+
+; TODO: enable testing for dwarf v5
+; RUN: llc -dwarf-version=5 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple= < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-5
+
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN: -dwarf-version=5 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple= < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-5
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-5
+
+; This test is derived from test/CodeGen/X86/dwarf-headers.ll
+
+; Looking for DWARF headers to be generated correctly.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)
+
+; Test case built from:
+;struct S {
+;  int s1;
+;};
+;
+;S s;
+
+; Verify the v4 non-split headers.
+; Note that we check the exact offset of the DIEs because that tells us
+; the length of the header.
+;
+; SINGLE-4: .debug_info contents:
+; SINGLE-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x000b: DW_TAG_compile_unit
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v4 split headers.
+;
+; O-4: .debug_info contents:
+; O-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; O-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_info.dwo contents:
+; DWO-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_types.dwo contents:
+; DWO-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v5 non-split headers. Type units come first.
+; All .debug_info sections are reported in one go, but the offset resets for
+; each new section.
+;
+; SINGLE-5: .debug_info contents:
+; SINGLE-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_type, abbr_offset
+; SINGLE-5: 0x0018: DW_TAG_type_unit
+; SINGLE-5-NOT: contents:
+; SINGLE-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_compile, abbr_offset
+; SINGLE-5: 0x000c: DW_TAG_compile_unit
+
+; Verify the v5 split headers.
+;
+; O-5: .debug_info contents:
+; O-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset
+; O-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; O-5: 0x0014: DW_TAG_skeleton_unit 
+;
+; DWO-5: .debug_info.dwo contents:
+; DWO-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_type, abbr_offset
+; DWO-5: 0x0018: DW_TAG_type_unit
+; DWO-5: 0x0033: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_compile, abbr_offset
+; DWO-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; DWO-5: 0x0047: DW_TAG_compile_unit
+
+
+; ModuleID = 't.cpp'
+source_filename = "t.cpp"
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown-wasm"
+
+%struct.S = type { i32 }
+
+@s = global %struct.S zeroinitializer, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11}
+!llvm.ident = !{!12}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = 

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff updated this revision to Diff 301133.
dschuff added a comment.

use GenericSectionID instead of ~0


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

Files:
  llvm/lib/MC/MCObjectFileInfo.cpp


Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -965,7 +965,7 @@
   utostr(Hash));
   case Triple::Wasm:
 return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
-   ~0);
+   MCContext::GenericSectionID);
   case Triple::MachO:
   case Triple::COFF:
   case Triple::GOFF:


Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -965,7 +965,7 @@
   utostr(Hash));
   case Triple::Wasm:
 return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
-   ~0);
+   MCContext::GenericSectionID);
   case Triple::MachO:
   case Triple::COFF:
   case Triple::GOFF:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added inline comments.



Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   ~0);

dschuff wrote:
> dblaikie wrote:
> > dschuff wrote:
> > > dblaikie wrote:
> > > > dschuff wrote:
> > > > > dschuff wrote:
> > > > > > I may add a couple more tests to this, but I did want to ask 
> > > > > > @sbc100 about this, since I'm not 100% sure at the uniqueID field 
> > > > > > is for.
> > > > > also let me be more clear about the question here: what is `UniqueID` 
> > > > > for, and is it bad that I'm just passing it a number that is totally 
> > > > > not unique?
> > > > For ELF, at least, I believe the unique ID is used to know which 
> > > > elements are to be treated as part of the same deduplication set.
> > > > 
> > > > If Wasm support in lld does the same thing, then using the same number 
> > > > for every type unit would mean the linked binary would end up with only 
> > > > one type definition - even when the input has many varied/independent 
> > > > type definitions. Likely not what's intended.
> > > For wasm I had thought that was what the 3rd argument (Group) was for. So 
> > > if that's what `UniqueID` is for, then I have the same question about 
> > > Group :)
> > Oh, fair enough - I hadn't read closely. Yeah, guess it's up to you 
> > folks/how the wasm object format works... - so I'm with you on the "what is 
> > the uniqueID field for" (& what's the other field that's taking the hash?) 
> > & I'll leave it to you folks to... hash out.
> @sbc100 ping, is this what we want here?
It looks like you can use GenericSectionID rather and ~0 here (and we should 
change that elsewhere too).

Other than that lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

still lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   ~0);

dblaikie wrote:
> dschuff wrote:
> > dblaikie wrote:
> > > dschuff wrote:
> > > > dschuff wrote:
> > > > > I may add a couple more tests to this, but I did want to ask @sbc100 
> > > > > about this, since I'm not 100% sure at the uniqueID field is for.
> > > > also let me be more clear about the question here: what is `UniqueID` 
> > > > for, and is it bad that I'm just passing it a number that is totally 
> > > > not unique?
> > > For ELF, at least, I believe the unique ID is used to know which elements 
> > > are to be treated as part of the same deduplication set.
> > > 
> > > If Wasm support in lld does the same thing, then using the same number 
> > > for every type unit would mean the linked binary would end up with only 
> > > one type definition - even when the input has many varied/independent 
> > > type definitions. Likely not what's intended.
> > For wasm I had thought that was what the 3rd argument (Group) was for. So 
> > if that's what `UniqueID` is for, then I have the same question about Group 
> > :)
> Oh, fair enough - I hadn't read closely. Yeah, guess it's up to you folks/how 
> the wasm object format works... - so I'm with you on the "what is the 
> uniqueID field for" (& what's the other field that's taking the hash?) & I'll 
> leave it to you folks to... hash out.
@sbc100 ping, is this what we want here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   ~0);

dschuff wrote:
> dblaikie wrote:
> > dschuff wrote:
> > > dschuff wrote:
> > > > I may add a couple more tests to this, but I did want to ask @sbc100 
> > > > about this, since I'm not 100% sure at the uniqueID field is for.
> > > also let me be more clear about the question here: what is `UniqueID` 
> > > for, and is it bad that I'm just passing it a number that is totally not 
> > > unique?
> > For ELF, at least, I believe the unique ID is used to know which elements 
> > are to be treated as part of the same deduplication set.
> > 
> > If Wasm support in lld does the same thing, then using the same number for 
> > every type unit would mean the linked binary would end up with only one 
> > type definition - even when the input has many varied/independent type 
> > definitions. Likely not what's intended.
> For wasm I had thought that was what the 3rd argument (Group) was for. So if 
> that's what `UniqueID` is for, then I have the same question about Group :)
Oh, fair enough - I hadn't read closely. Yeah, guess it's up to you folks/how 
the wasm object format works... - so I'm with you on the "what is the uniqueID 
field for" (& what's the other field that's taking the hash?) & I'll leave it 
to you folks to... hash out.



Comment at: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll:24-30
+; Looking for DWARF headers to be generated correctly.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)

dschuff wrote:
> dblaikie wrote:
> > Given that S.plit DWARF type units don't require comdat support (the dwp 
> > tool will be aware of the type units and parse their boundaries, read their 
> > type hash from the TU Header, etc). /may/ be worth separating that 
> > functionality/testing from the comdat support/testing - but maybe not all 
> > that interesting to separate it into two separate patches. (& if you find 
> > the test coverage works better in one test, that's OK - just a thought)
> I copied this test more-or-less directly from X86 :)
> Although I will say it's kind of nice to test all the header types in one go 
> like this just because it's slightly annoying to construct the LLVM metadata 
> for debuginfo tests, so it's nice to be able to share that as much as 
> possible. Previously we just didn't have enough coverage for split-dwarf 
> anyway.
Ah, fair enough - symmetry sounds good :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   ~0);

dblaikie wrote:
> dschuff wrote:
> > dschuff wrote:
> > > I may add a couple more tests to this, but I did want to ask @sbc100 
> > > about this, since I'm not 100% sure at the uniqueID field is for.
> > also let me be more clear about the question here: what is `UniqueID` for, 
> > and is it bad that I'm just passing it a number that is totally not unique?
> For ELF, at least, I believe the unique ID is used to know which elements are 
> to be treated as part of the same deduplication set.
> 
> If Wasm support in lld does the same thing, then using the same number for 
> every type unit would mean the linked binary would end up with only one type 
> definition - even when the input has many varied/independent type 
> definitions. Likely not what's intended.
For wasm I had thought that was what the 3rd argument (Group) was for. So if 
that's what `UniqueID` is for, then I have the same question about Group :)



Comment at: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll:24-30
+; Looking for DWARF headers to be generated correctly.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)

dblaikie wrote:
> Given that S.plit DWARF type units don't require comdat support (the dwp tool 
> will be aware of the type units and parse their boundaries, read their type 
> hash from the TU Header, etc). /may/ be worth separating that 
> functionality/testing from the comdat support/testing - but maybe not all 
> that interesting to separate it into two separate patches. (& if you find the 
> test coverage works better in one test, that's OK - just a thought)
I copied this test more-or-less directly from X86 :)
Although I will say it's kind of nice to test all the header types in one go 
like this just because it's slightly annoying to construct the LLVM metadata 
for debuginfo tests, so it's nice to be able to share that as much as possible. 
Previously we just didn't have enough coverage for split-dwarf anyway.



Comment at: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll:47
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset

dblaikie wrote:
> aardappel wrote:
> > I guess this doesn't actually test that only one copy of these remain after 
> > linking? That's already tested in LLD?
> Certainly lld shouldn't be tested here, no (llvm tests can't depend on other 
> subprojects like lld) - but yeah, some kind of comdat deduplication tests 
> should exist in lld (they don't have to be DWARF specific - the DWARF type 
> deduplication is meant to piggy-back on the existing comdat deduplication 
> infrastructure in thel inker)
@aardappel yes, pretty much what @dblaikie said :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   ~0);

dschuff wrote:
> dschuff wrote:
> > I may add a couple more tests to this, but I did want to ask @sbc100 about 
> > this, since I'm not 100% sure at the uniqueID field is for.
> also let me be more clear about the question here: what is `UniqueID` for, 
> and is it bad that I'm just passing it a number that is totally not unique?
For ELF, at least, I believe the unique ID is used to know which elements are 
to be treated as part of the same deduplication set.

If Wasm support in lld does the same thing, then using the same number for 
every type unit would mean the linked binary would end up with only one type 
definition - even when the input has many varied/independent type definitions. 
Likely not what's intended.



Comment at: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll:24-30
+; Looking for DWARF headers to be generated correctly.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)

Given that S.plit DWARF type units don't require comdat support (the dwp tool 
will be aware of the type units and parse their boundaries, read their type 
hash from the TU Header, etc). /may/ be worth separating that 
functionality/testing from the comdat support/testing - but maybe not all that 
interesting to separate it into two separate patches. (& if you find the test 
coverage works better in one test, that's OK - just a thought)



Comment at: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll:47
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset

aardappel wrote:
> I guess this doesn't actually test that only one copy of these remain after 
> linking? That's already tested in LLD?
Certainly lld shouldn't be tested here, no (llvm tests can't depend on other 
subprojects like lld) - but yeah, some kind of comdat deduplication tests 
should exist in lld (they don't have to be DWARF specific - the DWARF type 
deduplication is meant to piggy-back on the existing comdat deduplication 
infrastructure in thel inker)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Wouter van Oortmerssen via Phabricator via cfe-commits]

aardappel added inline comments.



Comment at: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll:47
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset

I guess this doesn't actually test that only one copy of these remain after 
linking? That's already tested in LLD?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   ~0);

dschuff wrote:
> I may add a couple more tests to this, but I did want to ask @sbc100 about 
> this, since I'm not 100% sure at the uniqueID field is for.
also let me be more clear about the question here: what is `UniqueID` for, and 
is it bad that I'm just passing it a number that is totally not unique?



Comment at: llvm/lib/MC/WasmObjectWriter.cpp:1353
   if (Begin) {
 WasmIndices[cast(Begin)] = CustomSections.size();
-if (SectionName != Begin->getName())

sbc100 wrote:
> Just checking in the case where you were hitting this error the SectionName 
> was duplicate but the `Begin` is uniquified ?
> 
> 
right. There's a second .debug_info section, so the name of `Begin` gets 
uniquified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Sam Clegg via Phabricator via cfe-commits]

sbc100 accepted this revision.
sbc100 added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/lib/MC/WasmObjectWriter.cpp:1353
   if (Begin) {
 WasmIndices[cast(Begin)] = CustomSections.size();
-if (SectionName != Begin->getName())

Just checking in the case where you were hitting this error the SectionName was 
duplicate but the `Begin` is uniquified ?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   ~0);

I may add a couple more tests to this, but I did want to ask @sbc100 about 
this, since I'm not 100% sure at the uniqueID field is for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff created this revision.
dschuff added reviewers: aardappel, sbc100.
Herald added subscribers: llvm-commits, cfe-commits, ecnelises, sunfish, 
hiraditya, jgravelle-google.
Herald added projects: clang, LLVM.
dschuff requested review of this revision.
Herald added subscribers: ormris, aheejin.

Since Wasm comdat sections work similarly to ELF, we can use that mechanism
to eliminate duplicate dwarf type information in the same way.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88603

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll

Index: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
@@ -0,0 +1,116 @@
+; RUN: llc -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-4
+
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN: -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-4
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-4
+
+; TODO: enable testing for dwarf v5
+; RUN: llc -dwarf-version=5 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple= < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-5
+
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN: -dwarf-version=5 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple= < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-5
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-5
+
+; This test is derived from test/CodeGen/X86/dwarf-headers.ll
+
+; Looking for DWARF headers to be generated correctly.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)
+
+; Test case built from:
+;struct S {
+;  int s1;
+;};
+;
+;S s;
+
+; Verify the v4 non-split headers.
+; Note that we check the exact offset of the DIEs because that tells us
+; the length of the header.
+;
+; SINGLE-4: .debug_info contents:
+; SINGLE-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x000b: DW_TAG_compile_unit
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v4 split headers.
+;
+; O-4: .debug_info contents:
+; O-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; O-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_info.dwo contents:
+; DWO-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_types.dwo contents:
+; DWO-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v5 non-split headers. Type units come first.
+; All .debug_info sections are reported in one go, but the offset resets for
+; each new section.
+;
+; SINGLE-5: .debug_info contents:
+; SINGLE-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_type, abbr_offset
+; SINGLE-5: 0x0018: DW_TAG_type_unit
+; SINGLE-5-NOT: contents:
+; SINGLE-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_compile, abbr_offset
+; SINGLE-5: 0x000c: DW_TAG_compile_unit
+
+; Verify the v5 split headers.
+;
+; O-5: .debug_info contents:
+; O-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset
+; O-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; O-5: 0x0014: DW_TAG_skeleton_unit 
+;
+; DWO-5: .debug_info.dwo contents:
+; DWO-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_type, abbr_offset
+; DWO-5: 0x0018: DW_TAG_type_unit
+; DWO-5: 0x0033: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_compile, abbr_offset
+; DWO-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; DWO-5: 0x0047: DW_TAG_compile_unit
+
+
+; ModuleID = 't.cpp'
+source_filename = "t.cpp"
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown-wasm"
+
+%struct.S = type { i32 }
+
+@s = global %struct.S zeroinitializer, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11}
+!llvm.ident = !{!12}
+
+!0 =