[PATCH] D149917: [lld][WebAssembly] Add --keep-section flag

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.

otherwise LGTM




Comment at: lld/wasm/Options.td:196
 
+defm keep_section: Eq<"keep-section",
+ "Preserve a section even when --strip-all is given.  This is useful for 
compiler drivers such as clang or emcc that, for example, depend on the 
features section for post-link processing.">;

dschuff wrote:
> Can this flag be used more than once? From the code it looks like maybe it 
> can, but the description should maybe say that and there should be a test 
> (probably I should have written one for objcopy/strip too)
Should this description also say that keep-section can be used more than once? 
or is there some other way to know that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149917

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

[PATCH] D149917: [lld][WebAssembly] Add --keep-section flag

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: lld/wasm/Options.td:196
 
+defm keep_section: Eq<"keep-section",
+ "Preserve a section even when --strip-all is given.  This is useful for 
compiler drivers such as clang or emcc that, for example, depend on the 
features section for post-link processing.">;

Can this flag be used more than once? From the code it looks like maybe it can, 
but the description should maybe say that and there should be a test (probably 
I should have written one for objcopy/strip too)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149917

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

[PATCH] D159383: [Headers] Remove musl-related comment about NULL

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Although having said all that, I guess a question for @aaron.ballman or other 
clang header experts: It does seem that many std C headers in clang are 
designed to handle this kind of case using `include_next` (e.g. stdint.h and 
stdatomic.h) but not all of them (e.g. stddef.h or stdbool.h). So maybe we 
should try to make them consistent anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159383

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

[PATCH] D159383: [Headers] Remove musl-related comment about NULL

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

The 2 common WebAssembly toolchain variants (Emscripten and wasi-sdk) use libcs 
that are derived from musl (a subset along with additions specific to their 
environments); they share the system-include configuration in 
`WebAssembly::AddClangSystemIncludeArgs()` in 
`clang/lib/Driver/Toolchains/WebAssembly.cpp`, which adds the ResourceDir 
before the Sysroot (and that I believe results in this include order). So we 
should maybe consider fixing that (something @aheejin and I should discuss with 
@sunfish and @sbc100 and the rest of the Emscripten and/or wasi folks).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159383

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

[PATCH] D159383: [Headers] Remove musl-related comment about NULL

[Derek Schuff via Phabricator via cfe-commits]

dschuff added subscribers: sbc100, sunfish.
dschuff added a comment.

The 2 common WebAssembly toolchain variants (Emscripten and wasi-sdk) use libcs 
that are derived from musl (a subset along with additions specific to their 
environments); they share the system-include configuration in 
`WebAssembly::AddClangSystemIncludeArgs()` in 
`clang/lib/Driver/Toolchains/WebAssembly.cpp`, which adds the ResourceDir 
before the Sysroot (and that I believe results in this include order). So we 
should maybe consider fixing that (something @aheejin and I should discuss with 
@sunfish and @sbc100 and the rest of the Emscripten and/or wasi folks).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159383

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

[PATCH] D159383: [Headers] Remove musl-related comment about NULL

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Suggested edit to the commit description:
"use musl and stddef.h at the same time" -> "use musl and clang's stddef.h at 
the same time"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159383

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

[PATCH] D78441: Delete NaCl support

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

In D78441#4602312 , @brad wrote:

> In D78441#4593287 , @dschuff wrote:
>
>> Deprecation is progressing 
>> (https://groups.google.com/a/chromium.org/g/chromium-extensions/c/v8H1UHnPotY/m/NmzrIv_VBAAJ)
>>  but we are still supporting it on some platforms, (and using clang's 
>> upstream support), so we aren't there yet.
>
> Ok, it's a bit confusing with different dates in different spots. So 
> according to that the mainstream OS (Win/macOS/Linux) releases of Chromium 
> will drop support by Dec 2023 just leaving ChromeOS support.

Correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78441

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

[PATCH] D78441: Delete NaCl support

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Deprecation is progressing 
(https://groups.google.com/a/chromium.org/g/chromium-extensions/c/v8H1UHnPotY/m/NmzrIv_VBAAJ)
 but we are still supporting it on some platforms, (and using clang's upstream 
support), so we aren't there yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78441

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

[PATCH] D150803: [WebAssembly] Support `annotate` clang attributes for marking functions.

[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 rG220fe00a7c0f: [WebAssembly] Support `annotate` clang 
attributes for marking functions. (authored by brendandahl, committed by 
dschuff).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150803

Files:
  lld/test/wasm/func-attr-tombstone.s
  lld/test/wasm/func-attr.s
  lld/test/wasm/merge-func-attr-section.s
  lld/wasm/InputChunks.cpp
  lld/wasm/InputFiles.cpp
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/BinaryFormat/WasmRelocs.def
  llvm/include/llvm/MC/MCExpr.h
  llvm/lib/MC/MCExpr.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/lib/Object/WasmObjectFile.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyWasmObjectWriter.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
  llvm/test/CodeGen/WebAssembly/func-attr-annotate.ll
  llvm/test/MC/WebAssembly/func-attr.s

Index: llvm/test/MC/WebAssembly/func-attr.s
===
--- /dev/null
+++ llvm/test/MC/WebAssembly/func-attr.s
@@ -0,0 +1,21 @@
+# RUN: llvm-mc -triple=wasm32-unknown-unknown < %s | FileCheck %s
+# Check that it also comiled to object for format.
+# RUN: llvm-mc -triple=wasm32-unknown-unknown -filetype=obj -o - < %s | obj2yaml | FileCheck -check-prefix=CHECK-OBJ %s
+
+foo:
+.globl foo
+.functype foo () -> ()
+end_function
+
+.section.custom_section.llvm.func_attr.custom0,"",@
+.int32  foo@FUNCINDEX
+
+# CHECK:   .section .custom_section.llvm.func_attr.custom0,"",@
+# CHECK-NEXT: .int32  foo@FUNCINDEX
+
+# CHECK-OBJ:- Type:CUSTOM
+# CHECK-OBJ-NEXT: Relocations:
+# CHECK-OBJ-NEXT:- Type:R_WASM_FUNCTION_INDEX_I32
+# CHECK-OBJ-NEXT:  Index:   0
+# CHECK-OBJ-NEXT:  Offset:  0x0
+# CHECK-OBJ-NEXT: Name:llvm.func_attr.custom0
Index: llvm/test/CodeGen/WebAssembly/func-attr-annotate.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/func-attr-annotate.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s -asm-verbose=false -wasm-keep-registers | FileCheck %s
+
+target triple = "wasm32-unknown-unknown"
+
+@.str = private unnamed_addr constant [8 x i8] c"custom0\00", section "llvm.metadata"
+@.str.1 = private unnamed_addr constant [7 x i8] c"main.c\00", section "llvm.metadata"
+@.str.2 = private unnamed_addr constant [8 x i8] c"custom1\00", section "llvm.metadata"
+@.str.3 = private unnamed_addr constant [8 x i8] c"custom2\00", section "llvm.metadata"
+@llvm.global.annotations = appending global [3 x { ptr, ptr, ptr, i32, ptr }] [{ ptr, ptr, ptr, i32, ptr } { ptr @test0, ptr @.str, ptr @.str.1, i32 4, ptr null }, { ptr, ptr, ptr, i32, ptr } { ptr @test1, ptr @.str, ptr @.str.1, i32 5, ptr null }, { ptr, ptr, ptr, i32, ptr } { ptr @test2, ptr @.str.2, ptr @.str.1, i32 6, ptr null }], section "llvm.metadata"
+
+define void @test0() {
+  ret void
+}
+
+define void @test1() {
+  ret void
+}
+
+define void @test2() {
+  ret void
+}
+
+define void @test3() {
+  ret void
+}
+
+; CHECK:  .section.custom_section.llvm.func_attr.annotate.custom0,"",@
+; CHECK-NEXT: .int32  test0@FUNCINDEX
+; CHECK-NEXT: .int32  test1@FUNCINDEX
+; CHECK:  .section.custom_section.llvm.func_attr.annotate.custom1,"",@
+; CHECK-NEXT: .int32  test2@FUNCINDEX
Index: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
===
--- llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
+++ llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
@@ -66,6 +66,7 @@
   void emitEndOfAsmFile(Module ) override;
   void EmitProducerInfo(Module );
   void EmitTargetFeatures(Module );
+  void EmitFunctionAttributes(Module );
   void emitSymbolType(const MCSymbolWasm *Sym);
   void emitGlobalVariable(const GlobalVariable *GV) override;
   void emitJumpTableInfo() override;
Index: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
@@ -27,6 +27,8 @@
 #include "WebAssemblyTargetMachine.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Analysis/ValueTracking.h"
 #include "llvm/BinaryFormat/Wasm.h"
 #include "llvm/CodeGen/Analysis.h"
 #include "llvm/CodeGen/AsmPrinter.h"
@@ -438,6 +440,7 @@
 
   EmitProducerInfo(M);
   EmitTargetFeatures(M);
+  EmitFunctionAttributes(M);
 }
 
 void WebAssemblyAsmPrinter::EmitProducerInfo(Module ) {
@@ -556,6 +559,49 @@
   OutStreamer->popSection();
 }
 
+void WebAssemblyAsmPrinter::EmitFunctionAttributes(Module ) {
+  

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

After local discussion, I guess we decided to leave this as-is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151820

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

[PATCH] D150803: Add a new `wasm_custom` clang attribute for marking functions.

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

oh also: please add the new relocation type to the document at 
https://github.com/WebAssembly/tool-conventions/blob/main/Linking.md


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150803

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

[PATCH] D150803: Add a new `wasm_custom` clang attribute for marking functions.

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

backend code looks pretty good. I'll defer to the clang experts for the clang 
part.




Comment at: lld/test/wasm/custom-undefine.s:17
+.type   bar,@function
+bar:
+.functype   bar () -> ()

I don't fully understand how this test is different from custom-attr.s. Bar is 
still defined, so the only thing I can see that's different is that it's not 
called. Is it relying on the linker GCing bar so that it's not defined? is the 
result different when bar is really undefined?



Comment at: lld/test/wasm/merge-custom-attr-section.ll:1
+; RUN: split-file %s %t
+; RUN: llc -filetype=obj %t/a.ll -o %t1.o

wow I didn't know this utility existed 勞



Comment at: lld/test/wasm/merge-custom-attr-section.ll:7
+
+; Ensure two custo funct_attr sections are concatenated together.
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150803

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

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

> Presumably this change of change makes most sense in the emscripten ChangeLog 
> right?   We don't tend to document emscripten-specific changes in the llvm 
> release notes do we?

Yes, I think so. Also I think it's more likely to be seen by users in the 
emscripten changelog


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151820

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

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

> As far as I can tell the only way this change could break XNNpack if 
> XNN_ALLOCATION_ALIGNMENT = 8 is wrongly set there... as long as that is the 
> correct value for XNN_ALLOCATION_ALIGNMENT I don't see how this change could 
> break it.  If XNN_ALLOCATION_ALIGNMENT is set wrongly this change might 
> expose that bug.. but it seems correct to me.

yeah, that's actually what my concern is. IIUC as written the code is asking 
for 8, but it's being masked by our value of BIGGEST_ALIGNMENT.

I suppose we should land this since I think we do want to have it match 
max_align_t. But it does make me wonder (again) whether our choice of ABI is 
correct here.
Can you also put something in the emscripten release notes about this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151820

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

[PATCH] D150803: [WebAssembly] Add a new `wasm_async` clang attribute for marking async functions.

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

In D150803#4350554 , @sbc100 wrote:

> This change looks really nice.  I like the new relocation type, I think we 
> would have had to add that sooner or later anyway.
>
> My only major concern is making this attribute available outside of 
> emscripten (i.e. wasm32-unknown-emscripten).  It seems like we should maybe 
> called it `em_async` or something like that?  And make it illegal on other 
> targets?
>
> @dschuff WDYT?

Hm, this is interesting because in the long term we plan to have stack 
switching in wasm, which could allow for similar async behavior that JSPI has, 
and could be useful in non-web systems. But that's a ways off. The file format 
we are generating with this CL will be used in emscripten sooner (and we may 
want to try to stabilize it some point, possibly before pure wasm stack 
switching is usable in non-web systems).
So overall I kind of feel like I could go either way on this. Curious if 
@sunfish has had any thoughts about async outside of emscripten.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150803

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

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

> I don't think it will since `__BIGGEST_ALIGNMENT__ >= 
> XNN_ALLOCATION_ALIGNMENT` will remain true after this change.. so this change 
> should have no effect on that code.

I meant that when `__BIGGEST_ALIGNMENT__ >= XNN_ALLOCATION_ALIGNMENT` (which 
was true before and will remain true), then XNNPack uses `__builtin_alloca()` 
as the implementation of `XNN_SIMD_ALLOCA` (which presumably is for allocating 
SIMD values). This change will reduce the alignment used by 
`__builtin_alloca()` from 16 to 8, such that (I think) it is no longer suitable 
for SIMD values.

Maybe this is a bug in XNNPack (they should maybe be using 
XNN_ALLOCATION_ALIGNMENT with a value suitable for SIMD?) but given that 
BIGGEST_ALIGNMENT and alloca seem to be intended for any base type (including 
SIMD) it wouldn't be surprising if someone else were depending on this too.

which... maybe this is just re-litigating the previous discussion, I don't 
know. I wonder at what point our ABI should be treating SIMD values as "normal" 
rather than rare.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151820

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

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a subscriber: tlively.
dschuff added a comment.

> I don't think it will change anything in that code since 
> `__BIGGEST_ALIGNMENT__ >= XNN_ALLOCATION_ALIGNMENT` will still hold true both 
> before and after this change (XNN_ALLOCATION_ALIGNMENT == 4 on wasm)

Right, that check causes XNN_ALLOCATION_ALIGNMENT to be ignored in favor of 
using clang's `_builtin_alloca()` which will be changed by this CL.
I seem to recall that @tlively and I spent a bunch of time with XNNpack chasing 
down some kind of subtle error that I suspect had to do with alignment, but 
maybe he remembers that better than I do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151820

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

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

I guess this is basically the C version of max_align_t so it should match.
but... this still has the potential to break things.
e.g. it will change the allocation in 
https://github.com/google/XNNPACK/blob/master/src/xnnpack/allocator.h#L66
ISTR that was one of the projects that had an issue with this the first time 
around?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151820

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

[PATCH] D150670: [WebAssembly] Disable generation of fshl/fshr for rotates

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

FWIW X86 seems to do something similar elsewhere in this file 
(https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGBuiltin.cpp#L985-L986),
 although it doesn't seem common otherwise. I think I'd be OK with this 
approach (and it does seem better than trying to mess with instcombine or a new 
TTI hook or something)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150670

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

[PATCH] D149917: [lld][WebAssembly] Add --preserve-features flag

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

or use `--keep-section` to match objcopy/strip? 
https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-objcopy/wasm/basic-keep.test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149917

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

[PATCH] D149917: [lld][WebAssembly] Add --preserve-features flag

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Yeah, I think that would work. Or maybe `--preserve-sections=sec1,sec2` since 
we might want to preserve multiple sections.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149917

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

[PATCH] D149917: [lld][WebAssembly] Add --preserve-features flag

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Do we want to make this any more general? In the future we might want to 
preserve other sections, e.g. passing optimization or profiling info from LLVM 
to Binaryen. Or maybe JSPI info?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149917

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

[PATCH] D78441: Delete NaCl support

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Sorry, no :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78441

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

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

This broke us in emscripten as well (building with trunk clang against a 
recent-but-not-trunk version of libcxx). I can test the fix if you want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203

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

[PATCH] D131217: [clang][WebAssembly] Pass `-Wl,-no-type-check` through to the MC layer

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

This looks good to me from the wasm perspective


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131217

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

[PATCH] D123763: [randstruct] Enforce using a designated init for a randomized struct

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

In D123763#3485836 , @sbc100 wrote:

> This new test has been failing on the emscripten builders.. seemingly ever 
> since it landed:

This uses a fairly old sysroot. I tried with a new version of libcxx, and it 
seems to be passing. So maybe this is depending on some new C++ that our old 
sysroot doesn't support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123763

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

[PATCH] D118573: [clang][WebAssemmbly] Call TargetInfo::adjust in derived method.

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

Looks OK to me. All of these options handle explicit overrides to the ABI which 
I guess just didn't work before but doesn't change any defaults. We should 
probably put something in the emscripten release notes with a list of the flags 
that now actually do something, just in case someone was accidentally using 
them before :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118573

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

[PATCH] D117888: [clang][driver][wasm] Support -stdlib=libstdc++ for WebAssembly

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

I haven't reviewed this yet, but since we got one of these before and never 
merged it (https://reviews.llvm.org/D101464) we should probably land one of 
these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117888

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

[PATCH] D118082: [clang][Driver] use DWARF4 for wasm

[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 rGd0d8d2d572cd: [clang][Driver] use DWARF4 for wasm (authored 
by dschuff).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118082

Files:
  clang/lib/Driver/ToolChains/WebAssembly.h
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -132,6 +132,13 @@
 // RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff -gcolumn-info \
 // RUN: 2>&1 | FileCheck -check-prefix=CI %s
 
+// WebAssembly.
+// WebAssembly should default to DWARF4.
+// RUN: %clang -### -c -g %s -target wasm32 2>&1 \
+// RUN: | FileCheck -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -g %s -target wasm64 2>&1 \
+// RUN: | FileCheck -check-prefix=G_DWARF4 %s
+
 // RUN: %clang -### -c -gdwarf-2 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
 //
Index: clang/lib/Driver/ToolChains/WebAssembly.h
===
--- clang/lib/Driver/ToolChains/WebAssembly.h
+++ clang/lib/Driver/ToolChains/WebAssembly.h
@@ -51,6 +51,7 @@
   bool hasBlocksRuntime() const override;
   bool SupportsProfiling() const override;
   bool HasNativeLLVMSupport() const override;
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
   void
   addClangTargetOptions(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ,


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -132,6 +132,13 @@
 // RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff -gcolumn-info \
 // RUN: 2>&1 | FileCheck -check-prefix=CI %s
 
+// WebAssembly.
+// WebAssembly should default to DWARF4.
+// RUN: %clang -### -c -g %s -target wasm32 2>&1 \
+// RUN: | FileCheck -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -g %s -target wasm64 2>&1 \
+// RUN: | FileCheck -check-prefix=G_DWARF4 %s
+
 // RUN: %clang -### -c -gdwarf-2 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
 //
Index: clang/lib/Driver/ToolChains/WebAssembly.h
===
--- clang/lib/Driver/ToolChains/WebAssembly.h
+++ clang/lib/Driver/ToolChains/WebAssembly.h
@@ -51,6 +51,7 @@
   bool hasBlocksRuntime() const override;
   bool SupportsProfiling() const override;
   bool HasNativeLLVMSupport() const override;
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
   void
   addClangTargetOptions(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118082: [clang][Driver] use DWARF4 for wasm

[Derek Schuff via Phabricator via cfe-commits]

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

fix diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118082

Files:
  clang/lib/Driver/ToolChains/WebAssembly.h
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -132,6 +132,13 @@
 // RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff -gcolumn-info \
 // RUN: 2>&1 | FileCheck -check-prefix=CI %s
 
+// WebAssembly.
+// WebAssembly should default to DWARF4.
+// RUN: %clang -### -c -g %s -target wasm32 2>&1 \
+// RUN: | FileCheck -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -g %s -target wasm64 2>&1 \
+// RUN: | FileCheck -check-prefix=G_DWARF4 %s
+
 // RUN: %clang -### -c -gdwarf-2 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
 //
Index: clang/lib/Driver/ToolChains/WebAssembly.h
===
--- clang/lib/Driver/ToolChains/WebAssembly.h
+++ clang/lib/Driver/ToolChains/WebAssembly.h
@@ -51,6 +51,7 @@
   bool hasBlocksRuntime() const override;
   bool SupportsProfiling() const override;
   bool HasNativeLLVMSupport() const override;
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
   void
   addClangTargetOptions(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ,


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -132,6 +132,13 @@
 // RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff -gcolumn-info \
 // RUN: 2>&1 | FileCheck -check-prefix=CI %s
 
+// WebAssembly.
+// WebAssembly should default to DWARF4.
+// RUN: %clang -### -c -g %s -target wasm32 2>&1 \
+// RUN: | FileCheck -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -g %s -target wasm64 2>&1 \
+// RUN: | FileCheck -check-prefix=G_DWARF4 %s
+
 // RUN: %clang -### -c -gdwarf-2 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
 //
Index: clang/lib/Driver/ToolChains/WebAssembly.h
===
--- clang/lib/Driver/ToolChains/WebAssembly.h
+++ clang/lib/Driver/ToolChains/WebAssembly.h
@@ -51,6 +51,7 @@
   bool hasBlocksRuntime() const override;
   bool SupportsProfiling() const override;
   bool HasNativeLLVMSupport() const override;
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
   void
   addClangTargetOptions(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118082: [clang][Driver] use DWARF4 for wasm

[Derek Schuff via Phabricator via cfe-commits]

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

just one blank line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118082

Files:
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -134,13 +134,11 @@
 
 // WebAssembly.
 // WebAssembly should default to DWARF4.
-// RUN: %clang -### -c -g %s -target wasm32 -gdwarf-4 2>&1 \
+// RUN: %clang -### -c -g %s -target wasm32 2>&1 \
 // RUN: | FileCheck -check-prefix=G_DWARF4 %s
-// RUN: %clang -### -c -g %s -target wasm64 -gdwarf-4 2>&1 \
+// RUN: %clang -### -c -g %s -target wasm64 2>&1 \
 // RUN: | FileCheck -check-prefix=G_DWARF4 %s
 
-
-
 // RUN: %clang -### -c -gdwarf-2 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
 //


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -134,13 +134,11 @@
 
 // WebAssembly.
 // WebAssembly should default to DWARF4.
-// RUN: %clang -### -c -g %s -target wasm32 -gdwarf-4 2>&1 \
+// RUN: %clang -### -c -g %s -target wasm32 2>&1 \
 // RUN: | FileCheck -check-prefix=G_DWARF4 %s
-// RUN: %clang -### -c -g %s -target wasm64 -gdwarf-4 2>&1 \
+// RUN: %clang -### -c -g %s -target wasm64 2>&1 \
 // RUN: | FileCheck -check-prefix=G_DWARF4 %s
 
-
-
 // RUN: %clang -### -c -gdwarf-2 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
 //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118082: [clang][Driver] use DWARF4 for wasm

[Derek Schuff via Phabricator via cfe-commits]

dschuff added subscribers: pfaffe, azakai.
dschuff added a comment.

Also +cc @pfaffe and @azakai

I'd like to land this soon to fix our emscripten tests for now. But we should 
talk about what the default should be. I'm curious if @sunfish has any any 
opinions on DWARF for their use cases and AFAIK lldb/chrome devtools is (or is 
almost?) ready for DWARF5 so maybe we can go back to defaulting to dwarf5 once 
we figure out exactly what to do for symbolizing stack traces of optimized 
builds (since Binaryen doesn't support DWARF5 yet, and may or may not soon).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118082

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

[PATCH] D118082: [clang][Driver] use DWARF4 for wasm

[Derek Schuff via Phabricator via cfe-commits]

dschuff created this revision.
dschuff added reviewers: aardappel, sunfish.
Herald added subscribers: wingo, jgravelle-google, sbc100.
dschuff requested review of this revision.
Herald added subscribers: cfe-commits, aheejin.
Herald added a project: clang.

Opt into the old default of DWARF4 for now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118082

Files:
  clang/lib/Driver/ToolChains/WebAssembly.h
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -132,6 +132,15 @@
 // RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff -gcolumn-info \
 // RUN: 2>&1 | FileCheck -check-prefix=CI %s
 
+// WebAssembly.
+// WebAssembly should default to DWARF4.
+// RUN: %clang -### -c -g %s -target wasm32 -gdwarf-4 2>&1 \
+// RUN: | FileCheck -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -g %s -target wasm64 -gdwarf-4 2>&1 \
+// RUN: | FileCheck -check-prefix=G_DWARF4 %s
+
+
+
 // RUN: %clang -### -c -gdwarf-2 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
 //
Index: clang/lib/Driver/ToolChains/WebAssembly.h
===
--- clang/lib/Driver/ToolChains/WebAssembly.h
+++ clang/lib/Driver/ToolChains/WebAssembly.h
@@ -51,6 +51,7 @@
   bool hasBlocksRuntime() const override;
   bool SupportsProfiling() const override;
   bool HasNativeLLVMSupport() const override;
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
   void
   addClangTargetOptions(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ,


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -132,6 +132,15 @@
 // RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff -gcolumn-info \
 // RUN: 2>&1 | FileCheck -check-prefix=CI %s
 
+// WebAssembly.
+// WebAssembly should default to DWARF4.
+// RUN: %clang -### -c -g %s -target wasm32 -gdwarf-4 2>&1 \
+// RUN: | FileCheck -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -g %s -target wasm64 -gdwarf-4 2>&1 \
+// RUN: | FileCheck -check-prefix=G_DWARF4 %s
+
+
+
 // RUN: %clang -### -c -gdwarf-2 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
 //
Index: clang/lib/Driver/ToolChains/WebAssembly.h
===
--- clang/lib/Driver/ToolChains/WebAssembly.h
+++ clang/lib/Driver/ToolChains/WebAssembly.h
@@ -51,6 +51,7 @@
   bool hasBlocksRuntime() const override;
   bool SupportsProfiling() const override;
   bool HasNativeLLVMSupport() const override;
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
   void
   addClangTargetOptions(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115893: [WebAssembly] Support clang -fwasm-exceptions for bitcode

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:37
 
-// Emscripten's asm.js-style exception handling
-cl::opt
-WasmEnableEmEH("enable-emscripten-cxx-exceptions",
-   cl::desc("WebAssembly Emscripten-style exception handling"),
-   cl::init(false));
-
-// Emscripten's asm.js-style setjmp/longjmp handling
-cl::opt WasmEnableEmSjLj(
-"enable-emscripten-sjlj",
-cl::desc("WebAssembly Emscripten-style setjmp/longjmp handling"),
-cl::init(false));
-
-// Exception handling using wasm EH instructions
-cl::opt WasmEnableEH("wasm-enable-eh",
-   cl::desc("WebAssembly exception handling"),
-   cl::init(false));
-
-// setjmp/longjmp handling using wasm EH instrutions
-cl::opt WasmEnableSjLj("wasm-enable-sjlj",
- cl::desc("WebAssembly setjmp/longjmp handling"),
- cl::init(false));
+extern cl::opt WasmEnableEmEH;   // asm.js-style EH
+extern cl::opt WasmEnableEmSjLj; // asm.js-style SjLJ

aheejin wrote:
> dschuff wrote:
> > We could put these declarations in WebAssemblyUtilities.h; then they 
> > wouldn't have to be duplicated here and in WebAssemblyMCAsmInfo.cpp
> Done. But as a result they are not within `WebAssembly` namespace, which I 
> think is actually better because we have been cluttering the `llvm` namespace 
> so far. And I needed to attach `WebAssembly::` to all the usages of these 
> variables in other places. In this file I just used `using WebAssembly::***`, 
> because there are too many usages of these options.
 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115893

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

[PATCH] D115893: [WebAssembly] Support clang -fwasm-exceptions for bitcode

[Derek Schuff via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:37
 
-// Emscripten's asm.js-style exception handling
-cl::opt
-WasmEnableEmEH("enable-emscripten-cxx-exceptions",
-   cl::desc("WebAssembly Emscripten-style exception handling"),
-   cl::init(false));
-
-// Emscripten's asm.js-style setjmp/longjmp handling
-cl::opt WasmEnableEmSjLj(
-"enable-emscripten-sjlj",
-cl::desc("WebAssembly Emscripten-style setjmp/longjmp handling"),
-cl::init(false));
-
-// Exception handling using wasm EH instructions
-cl::opt WasmEnableEH("wasm-enable-eh",
-   cl::desc("WebAssembly exception handling"),
-   cl::init(false));
-
-// setjmp/longjmp handling using wasm EH instrutions
-cl::opt WasmEnableSjLj("wasm-enable-sjlj",
- cl::desc("WebAssembly setjmp/longjmp handling"),
- cl::init(false));
+extern cl::opt WasmEnableEmEH;   // asm.js-style EH
+extern cl::opt WasmEnableEmSjLj; // asm.js-style SjLJ

We could put these declarations in WebAssemblyUtilities.h; then they wouldn't 
have to be duplicated here and in WebAssemblyMCAsmInfo.cpp



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:357
+  // Before checking, we make sure TargetOptions.ExceptionModel is the same as
+  // MCAsmInfo.ExceptionsType. Normally when these have to be the same, because
+  // clang stores the exception model info in LangOptions, which is later




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115893

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

[PATCH] D107613: [Clang][DiagnosticSemaKinds] combine diagnostic texts

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

It looks like this error is intended to catch mismatches for attributes that 
can affect codegen such as noreturn (in which case it makes sense to have it as 
an error) but it also now fires for cases such as `__attribute__(warning())` 
which often do not duplicate the attribute on the definition. Is that intended?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107613

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

[PATCH] D107685: [WebAssembly] Tidy up EH/SjLj options

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added inline comments.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:449
+  // done in WasmEHPrepare pass after these IR passes, but Wasm SjLj requires
+  // Emscripten libraries and processed together in LowerEmscriptenEHSjLJ pass.
+  if (WasmEnableEmEH || WasmEnableEmSjLj || WasmEnableSjLj)

aheejin wrote:
> dschuff wrote:
> > it's not clear what "and processed" is intended to mean here.
> Yeah the sentence is unclear and even grammatically incorrect... Rewrote it.
LGTM now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107685

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

[PATCH] D107685: [WebAssembly] Tidy up EH/SjLj options

[Derek Schuff via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp:278
+// Two SjLj modes cannot be turned on at the same time
+assert(!(EnableEmSjLj && EnableWasmSjLj));
+// Wasm SjLj should be only used with Wasm EH

You could put the comment text directly in the assert, i.e. `assert(condition 
&& "text")`



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:48
 
+// Wasm exception handling using wasm EH instructions
+cl::opt WasmEnableEH("wasm-enable-eh",

this description seems a bit redundant. I guess "C++ exception handling using 
wasm EH instructions" might be a bit too specific since it could be anything 
that generates WinEH-style IR. Maybe just leave out the first "wasm"?



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:449
+  // done in WasmEHPrepare pass after these IR passes, but Wasm SjLj requires
+  // Emscripten libraries and processed together in LowerEmscriptenEHSjLJ pass.
+  if (WasmEnableEmEH || WasmEnableEmSjLj || WasmEnableSjLj)

it's not clear what "and processed" is intended to mean here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107685

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

[PATCH] D105749: WebAssembly: Update datalayout to match fp128 ABI change

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/test/CodeGen/WebAssembly/varargs.ll:5
 
-target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-target triple = "wasm32-unknown-unknown"
+target triple = "wasm32-unknown-emscripten"
 

sunfish wrote:
> It appears this change means that we're no longer testing varargs on 
> wasm32-unknown-unknown. Please update this test so that it tests both triples.
Done in rGd4e2693a679927a62dd738dd3bba24863dcd290a


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105749

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

[PATCH] D105749: WebAssembly: Update datalayout to match fp128 ABI change

[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 rGac02baab48c2: WebAssembly: Update datalayout to match fp128 
ABI change (authored by dschuff).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105749

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/varargs.ll

Index: llvm/test/CodeGen/WebAssembly/varargs.ll
===
--- llvm/test/CodeGen/WebAssembly/varargs.ll
+++ llvm/test/CodeGen/WebAssembly/varargs.ll
@@ -2,8 +2,7 @@
 
 ; Test varargs constructs.
 
-target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-target triple = "wasm32-unknown-unknown"
+target triple = "wasm32-unknown-emscripten"
 
 ; Test va_start.
 
@@ -167,21 +166,19 @@
 ; within a vararg buffer.
 
 ; CHECK-LABEL: call_fp128_alignment:
-; CHECK:  global.get  $push7=, __stack_pointer
-; CHECK-NEXT: i32.const   $push8=, 32
-; CHECK-NEXT: i32.sub $push12=, $pop7, $pop8
-; CHECK-NEXT: local.tee   $push11=, $1=, $pop12
-; CHECK-NEXT: global.set  __stack_pointer, $pop11
-; CHECK-NEXT: i32.const   $push0=, 24
+; CHECK:  global.get  $push5=, __stack_pointer
+; CHECK-NEXT: i32.const   $push6=, 32
+; CHECK-NEXT: i32.sub $push10=, $pop5, $pop6
+; CHECK-NEXT: local.tee   $push9=, $1=, $pop10
+; CHECK-NEXT: global.set  __stack_pointer, $pop9
+; CHECK-NEXT: i32.const   $push0=, 16
 ; CHECK-NEXT: i32.add $push1=, $1, $pop0
 ; CHECK-NEXT: i64.const   $push2=, -9223372036854775808
 ; CHECK-NEXT: i64.store   0($pop1), $pop2
-; CHECK-NEXT: i32.const   $push3=, 16
-; CHECK-NEXT: i32.add $push4=, $1, $pop3
-; CHECK-NEXT: i64.const   $push5=, 1
-; CHECK-NEXT: i64.store   0($pop4), $pop5
-; CHECK-NEXT: i32.const   $push6=, 7
-; CHECK-NEXT: i32.store   0($1), $pop6
+; CHECK-NEXT: i64.const   $push3=, 1
+; CHECK-NEXT: i64.store   8($1), $pop3
+; CHECK-NEXT: i32.const   $push4=, 7
+; CHECK-NEXT: i32.store   0($1), $pop4
 ; CHECK-NEXT: callcallee, $1
 define void @call_fp128_alignment(i8* %p) {
 entry:
Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -120,12 +120,17 @@
 const Target , const Triple , StringRef CPU, StringRef FS,
 const TargetOptions , Optional RM,
 Optional CM, CodeGenOpt::Level OL, bool JIT)
-: LLVMTargetMachine(T,
-TT.isArch64Bit()
-? "e-m:e-p:64:64-i64:64-n32:64-S128-ni:1"
-: "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1",
-TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
-getEffectiveCodeModel(CM, CodeModel::Large), OL),
+: LLVMTargetMachine(
+  T,
+  TT.isArch64Bit()
+  ? (TT.isOSEmscripten()
+ ? "e-m:e-p:64:64-i64:64-f128:64-n32:64-S128-ni:1"
+ : "e-m:e-p:64:64-i64:64-n32:64-S128-ni:1")
+  : (TT.isOSEmscripten()
+ ? "e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1"
+ : "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1"),
+  TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
+  getEffectiveCodeModel(CM, CodeModel::Large), OL),
   TLOF(new WebAssemblyTargetObjectFile()) {
   // WebAssembly type-checks instructions, but a noreturn function with a return
   // type that doesn't match the context will cause a check failure. So we lower
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -149,7 +149,10 @@
   explicit WebAssembly32TargetInfo(const llvm::Triple ,
const TargetOptions )
   : WebAssemblyTargetInfo(T, Opts) {
-resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:1");
+if (T.isOSEmscripten())
+  resetDataLayout("e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1");
+else
+  resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:1");
   }
 
 protected:
@@ -168,7 +171,10 @@
 SizeType = UnsignedLong;
 PtrDiffType = SignedLong;
 IntPtrType = SignedLong;
-resetDataLayout("e-m:e-p:64:64-i64:64-n32:64-S128-ni:1");
+if (T.isOSEmscripten())
+  resetDataLayout("e-m:e-p:64:64-i64:64-f128:64-n32:64-S128-ni:1");
+else
+  resetDataLayout("e-m:e-p:64:64-i64:64-n32:64-S128-ni:1");
   }
 
 protected:
___
cfe-commits mailing list

[PATCH] D105749: WebAssembly: Update datalayout to match fp128 ABI change

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: clang/lib/Basic/Targets/WebAssembly.h:175
+if (T.isOSEmscripten())
+  resetDataLayout("e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1");
+else

kripken wrote:
> Should this not be
> 
> resetDataLayout("e-m:e-p:64:64-i64:64-f128:64-n32:64-S128-ni:1");
> 
> (that is, the first two numbers should be 64?)
oops yes, that's a copypasta


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105749

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

[PATCH] D105749: WebAssembly: Update datalayout to match fp128 ABI change

[Derek Schuff via Phabricator via cfe-commits]

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

- fix wasm64 copypasta


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105749

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/varargs.ll

Index: llvm/test/CodeGen/WebAssembly/varargs.ll
===
--- llvm/test/CodeGen/WebAssembly/varargs.ll
+++ llvm/test/CodeGen/WebAssembly/varargs.ll
@@ -2,8 +2,7 @@
 
 ; Test varargs constructs.
 
-target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-target triple = "wasm32-unknown-unknown"
+target triple = "wasm32-unknown-emscripten"
 
 ; Test va_start.
 
@@ -167,21 +166,19 @@
 ; within a vararg buffer.
 
 ; CHECK-LABEL: call_fp128_alignment:
-; CHECK:  global.get  $push7=, __stack_pointer
-; CHECK-NEXT: i32.const   $push8=, 32
-; CHECK-NEXT: i32.sub $push12=, $pop7, $pop8
-; CHECK-NEXT: local.tee   $push11=, $1=, $pop12
-; CHECK-NEXT: global.set  __stack_pointer, $pop11
-; CHECK-NEXT: i32.const   $push0=, 24
+; CHECK:  global.get  $push5=, __stack_pointer
+; CHECK-NEXT: i32.const   $push6=, 32
+; CHECK-NEXT: i32.sub $push10=, $pop5, $pop6
+; CHECK-NEXT: local.tee   $push9=, $1=, $pop10
+; CHECK-NEXT: global.set  __stack_pointer, $pop9
+; CHECK-NEXT: i32.const   $push0=, 16
 ; CHECK-NEXT: i32.add $push1=, $1, $pop0
 ; CHECK-NEXT: i64.const   $push2=, -9223372036854775808
 ; CHECK-NEXT: i64.store   0($pop1), $pop2
-; CHECK-NEXT: i32.const   $push3=, 16
-; CHECK-NEXT: i32.add $push4=, $1, $pop3
-; CHECK-NEXT: i64.const   $push5=, 1
-; CHECK-NEXT: i64.store   0($pop4), $pop5
-; CHECK-NEXT: i32.const   $push6=, 7
-; CHECK-NEXT: i32.store   0($1), $pop6
+; CHECK-NEXT: i64.const   $push3=, 1
+; CHECK-NEXT: i64.store   8($1), $pop3
+; CHECK-NEXT: i32.const   $push4=, 7
+; CHECK-NEXT: i32.store   0($1), $pop4
 ; CHECK-NEXT: callcallee, $1
 define void @call_fp128_alignment(i8* %p) {
 entry:
Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -120,12 +120,17 @@
 const Target , const Triple , StringRef CPU, StringRef FS,
 const TargetOptions , Optional RM,
 Optional CM, CodeGenOpt::Level OL, bool JIT)
-: LLVMTargetMachine(T,
-TT.isArch64Bit()
-? "e-m:e-p:64:64-i64:64-n32:64-S128-ni:1"
-: "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1",
-TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
-getEffectiveCodeModel(CM, CodeModel::Large), OL),
+: LLVMTargetMachine(
+  T,
+  TT.isArch64Bit()
+  ? (TT.isOSEmscripten()
+ ? "e-m:e-p:64:64-i64:64-f128:64-n32:64-S128-ni:1"
+ : "e-m:e-p:64:64-i64:64-n32:64-S128-ni:1")
+  : (TT.isOSEmscripten()
+ ? "e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1"
+ : "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1"),
+  TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
+  getEffectiveCodeModel(CM, CodeModel::Large), OL),
   TLOF(new WebAssemblyTargetObjectFile()) {
   // WebAssembly type-checks instructions, but a noreturn function with a return
   // type that doesn't match the context will cause a check failure. So we lower
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -149,7 +149,10 @@
   explicit WebAssembly32TargetInfo(const llvm::Triple ,
const TargetOptions )
   : WebAssemblyTargetInfo(T, Opts) {
-resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:1");
+if (T.isOSEmscripten())
+  resetDataLayout("e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1");
+else
+  resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:1");
   }
 
 protected:
@@ -168,7 +171,10 @@
 SizeType = UnsignedLong;
 PtrDiffType = SignedLong;
 IntPtrType = SignedLong;
-resetDataLayout("e-m:e-p:64:64-i64:64-n32:64-S128-ni:1");
+if (T.isOSEmscripten())
+  resetDataLayout("e-m:e-p:64:64-i64:64-f128:64-n32:64-S128-ni:1");
+else
+  resetDataLayout("e-m:e-p:64:64-i64:64-n32:64-S128-ni:1");
   }
 
 protected:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105749: WebAssembly: Update datalayout to match fp128 ABI change

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a reviewer: sbc100.
dschuff added a comment.

This fix should go in ASAP (or else we should revert 
d1a96e906cc03a95cfd41a1f22bdda92651250c7 
) to fix 
the mismatch between LLVM and clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105749

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

[PATCH] D105749: WebAssembly: Update datalayout to match fp128 ABI change

[Derek Schuff via Phabricator via cfe-commits]

dschuff created this revision.
dschuff added a reviewer: azakai.
Herald added subscribers: sunfish, hiraditya, jgravelle-google, sbc100.
dschuff requested review of this revision.
Herald added subscribers: cfe-commits, aheejin.
Herald added projects: clang, LLVM.

This fix goes along with d1a96e906cc03a95cfd41a1f22bdda92651250c7 

and makes the fp128 alignment match clang's long double alignment.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105749

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/varargs.ll

Index: llvm/test/CodeGen/WebAssembly/varargs.ll
===
--- llvm/test/CodeGen/WebAssembly/varargs.ll
+++ llvm/test/CodeGen/WebAssembly/varargs.ll
@@ -2,8 +2,7 @@
 
 ; Test varargs constructs.
 
-target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-target triple = "wasm32-unknown-unknown"
+target triple = "wasm32-unknown-emscripten"
 
 ; Test va_start.
 
@@ -167,21 +166,19 @@
 ; within a vararg buffer.
 
 ; CHECK-LABEL: call_fp128_alignment:
-; CHECK:  global.get  $push7=, __stack_pointer
-; CHECK-NEXT: i32.const   $push8=, 32
-; CHECK-NEXT: i32.sub $push12=, $pop7, $pop8
-; CHECK-NEXT: local.tee   $push11=, $1=, $pop12
-; CHECK-NEXT: global.set  __stack_pointer, $pop11
-; CHECK-NEXT: i32.const   $push0=, 24
+; CHECK:  global.get  $push5=, __stack_pointer
+; CHECK-NEXT: i32.const   $push6=, 32
+; CHECK-NEXT: i32.sub $push10=, $pop5, $pop6
+; CHECK-NEXT: local.tee   $push9=, $1=, $pop10
+; CHECK-NEXT: global.set  __stack_pointer, $pop9
+; CHECK-NEXT: i32.const   $push0=, 16
 ; CHECK-NEXT: i32.add $push1=, $1, $pop0
 ; CHECK-NEXT: i64.const   $push2=, -9223372036854775808
 ; CHECK-NEXT: i64.store   0($pop1), $pop2
-; CHECK-NEXT: i32.const   $push3=, 16
-; CHECK-NEXT: i32.add $push4=, $1, $pop3
-; CHECK-NEXT: i64.const   $push5=, 1
-; CHECK-NEXT: i64.store   0($pop4), $pop5
-; CHECK-NEXT: i32.const   $push6=, 7
-; CHECK-NEXT: i32.store   0($1), $pop6
+; CHECK-NEXT: i64.const   $push3=, 1
+; CHECK-NEXT: i64.store   8($1), $pop3
+; CHECK-NEXT: i32.const   $push4=, 7
+; CHECK-NEXT: i32.store   0($1), $pop4
 ; CHECK-NEXT: callcallee, $1
 define void @call_fp128_alignment(i8* %p) {
 entry:
Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -120,12 +120,17 @@
 const Target , const Triple , StringRef CPU, StringRef FS,
 const TargetOptions , Optional RM,
 Optional CM, CodeGenOpt::Level OL, bool JIT)
-: LLVMTargetMachine(T,
-TT.isArch64Bit()
-? "e-m:e-p:64:64-i64:64-n32:64-S128-ni:1"
-: "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1",
-TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
-getEffectiveCodeModel(CM, CodeModel::Large), OL),
+: LLVMTargetMachine(
+  T,
+  TT.isArch64Bit()
+  ? (TT.isOSEmscripten()
+ ? "e-m:e-p:64:64-i64:64-f128:64-n32:64-S128-ni:1"
+ : "e-m:e-p:64:64-i64:64-n32:64-S128-ni:1")
+  : (TT.isOSEmscripten()
+ ? "e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1"
+ : "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1"),
+  TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
+  getEffectiveCodeModel(CM, CodeModel::Large), OL),
   TLOF(new WebAssemblyTargetObjectFile()) {
   // WebAssembly type-checks instructions, but a noreturn function with a return
   // type that doesn't match the context will cause a check failure. So we lower
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -149,7 +149,10 @@
   explicit WebAssembly32TargetInfo(const llvm::Triple ,
const TargetOptions )
   : WebAssemblyTargetInfo(T, Opts) {
-resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:1");
+if (T.isOSEmscripten())
+  resetDataLayout("e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1");
+else
+  resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:1");
   }
 
 protected:
@@ -168,7 +171,10 @@
 SizeType = UnsignedLong;
 PtrDiffType = SignedLong;
 IntPtrType = SignedLong;
-resetDataLayout("e-m:e-p:64:64-i64:64-n32:64-S128-ni:1");
+if (T.isOSEmscripten())
+  resetDataLayout("e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1");
+else
+  

[PATCH] D102791: [WebAssembly] Warn on exception spec for Emscripten EH

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.

Got it, this makes sense to me too. And since it can be turned off, I'm not too 
worried about annoying users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102791

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

[PATCH] D102791: [WebAssembly] Warn on exception spec for Emscripten EH

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

BTW Is there a way to disable this warning?
Since IIUC this could cause code that was not warning (because it used 
-fno-exceptions or used emscripten's default of -fignore-exceptions) to now 
start warning, sometimes that makes users who use -Werror unhappy.




Comment at: clang/lib/CodeGen/CGException.cpp:495
+CGM.getLangOpts().getExceptionHandling() ==
+LangOptions::ExceptionHandlingKind::None &&
+EST == EST_Dynamic)

Does emscripten's default of `-fignore-exceptions` also end up as haveing 
`ExceptionHandlingKind::None` even though exceptions aren't disabled?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102791

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

[PATCH] D101112: [WebAssembly] Finalize wasm_simd128.h intrinsics

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: clang/lib/Headers/wasm_simd128.h:171
+
+#define wasm_v128_load8_lane(__ptr, __vec, __i)
\
+  ((v128_t)__builtin_wasm_load8_lane((signed char *)(__ptr), (__i8x16)(__vec), 
\

tlively wrote:
> aheejin wrote:
> > dschuff wrote:
> > > out of curiosity, why are these macros, while all the rest (including 
> > > ones that don't need declarations such as `wasm_i64x2_eq`) seem to be 
> > > inline functions?
> > I was also curious about this too.
> The `i` parameter needs to be an integer constant, and I never figured out a 
> way to enforce that for a function parameter. (But using a macro works 
> because the codegen for the builtin functions can error out on non-constant 
> arguments.)
Ah, that makes sense. It does make me wonder, do we have any documentation 
about those constraints? I guess this file itself is more-or-less what we have, 
right? If the constraint is violated, is the error from the compiler 
intelligible?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101112

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

[PATCH] D101112: [WebAssembly] Finalize wasm_simd128.h intrinsics

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: clang/lib/Headers/wasm_simd128.h:171
+
+#define wasm_v128_load8_lane(__ptr, __vec, __i)
\
+  ((v128_t)__builtin_wasm_load8_lane((signed char *)(__ptr), (__i8x16)(__vec), 
\

out of curiosity, why are these macros, while all the rest (including ones that 
don't need declarations such as `wasm_i64x2_eq`) seem to be inline functions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101112

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

[PATCH] D100981: Delete le32/le64 targets

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Would it make sense for you to to upstream an LLVM target such as le32-halide? 
(Or perhaps even arm32-halide or some other?) Then you'd actually have more 
control over your own codegen, datalayout, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100981

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

[PATCH] D100981: Delete le32/le64 targets

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

Thanks. I had heard in the past that there were some other folks who had used 
le32/le64 as a "generic" target (in fact that's why it's named so generically, 
rather than being called "pnacl" or similar) but I haven't heard of anything 
recently, and as you can see nobody has upstreamed any support for other OS or 
target specializations or asked to collaborate on it. Practically speaking even 
a target that wants fairly generic bitcode would probably want its own triple, 
so unless this removal captures someone's attention who wants to keep 
maintaining it, this should be fine to remove.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100981

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

[PATCH] D99623: [WebAssembly] Implement i64x2 comparisons

[Derek Schuff via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:218
 
-TARGET_BUILTIN(__builtin_wasm_eq_i64x2, "V2LLiV2LLiV2LLi", "nc", "simd128")
-

Is the builtin/intrinsic wrong now? Or just not necessary because we can use 
builtin operators?



Comment at: llvm/test/CodeGen/WebAssembly/simd-select.ll:313
 ; CHECK-NEXT:# fallthrough-return
 <2 x i64> %x, <2 x i64> %y) {
   %c = icmp slt <2 x i64> %a, %b

pre-existing, but is there a reason why the CHECKs here are in the middle of 
the IR function signature?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99623

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

[PATCH] D98466: [WebAssembly] Remove experimental SIMD instructions

[Derek Schuff via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:191
 
-TARGET_BUILTIN(__builtin_wasm_qfma_f32x4, "V4fV4fV4fV4f", "nc", "simd128")
-TARGET_BUILTIN(__builtin_wasm_qfms_f32x4, "V4fV4fV4fV4f", "nc", "simd128")

I guess this answers my question from the previous review :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98466

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

[PATCH] D98457: [WebAssembly] Remove unimplemented-simd target features

[Derek Schuff via Phabricator via cfe-commits]

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



Comment at: clang/docs/ClangCommandLineReference.rst:3801
 Pass -z  to the linker
-

extraneous change?



Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:191
 
-TARGET_BUILTIN(__builtin_wasm_qfma_f32x4, "V4fV4fV4fV4f", "nc", 
"unimplemented-simd128")
-TARGET_BUILTIN(__builtin_wasm_qfms_f32x4, "V4fV4fV4fV4f", "nc", 
"unimplemented-simd128")
-TARGET_BUILTIN(__builtin_wasm_qfma_f64x2, "V2dV2dV2dV2d", "nc", 
"unimplemented-simd128")
-TARGET_BUILTIN(__builtin_wasm_qfms_f64x2, "V2dV2dV2dV2d", "nc", 
"unimplemented-simd128")
+TARGET_BUILTIN(__builtin_wasm_qfma_f32x4, "V4fV4fV4fV4f", "nc", "simd128")
+TARGET_BUILTIN(__builtin_wasm_qfms_f32x4, "V4fV4fV4fV4f", "nc", "simd128")

is QFMA actually in MVP simd? I thought it was non-deterministic?



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1712
 };
-  } else if (NumConstantLanes >= NumSplatLanes &&
- Subtarget->hasUnimplementedSIMD128()) {
-// If we support v128.const, emit it directly
+  } else if (NumConstantLanes >= NumSplatLanes) {
 SmallVector ConstLanes;

out of curiosity, are there any cases where a splat could be smaller than a 
v128 const, since the consts are pretty big?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98457

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

[PATCH] D97834: [WebAssembly] Disable uses of __clang_call_terminate

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.

I agree this is a good approach, and I also like that it's smaller and simpler. 
In the future we can revisit whether following this particular Itanium 
convention buys us anything useful or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97834

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

[PATCH] D94039: [WebAssembly] Update WasmEHPrepare for the new spec

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/lib/CodeGen/WasmEHPrepare.cpp:374-375
+  // be lowered to wasm 'catch' instruction. We do this mainly because
+  // instruction selection cannot handle wasm.get.exception intrinsic's token
+  // argument.
+  Instruction *CatchCI =

tlively wrote:
> What is the token argument used for? Could clang generate `llvm.wasm.catch` 
> directly?
Token arguments (https://llvm.org/docs/ExceptionHandling.html#wineh) are used 
to preserve the original scoping structure in mid-level optimization passes. I 
haven't looked at this intrinsic recently but I'd guess that maybe the token 
keeps it from getting moved out of its containing scope?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94039

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

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

One other question then: do you know if Debian and/or Ubuntu still have the 
same support for running x32 programs on the regular x86-64 distribution? 
(presumably yes, since you aren't changing the existing behavior).
AFAIK clang's current support was developed against Ubuntu, but I haven't tried 
it in a long time and to my knowledge nobody has submitted any patches for x32 
in a while either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D52050

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

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Can you upload the diff with full context (e.g. use `diff -U ` or use 
arcanist to upload)?

I'm a bit confused; the commit message talks about X32 being a separate 
architecture, but you're not adding any new architecture triples here (it still 
uses x86_64 as the architecture and selects the ABI via the environment). 
AFAICS what really changes is that the structure of the include and lib 
directories is now multi-arch style with the x32 headers alongside the x86_64 
base headers?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D52050

___
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

[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

[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

[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

[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

[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 = 

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

[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=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  

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

[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 = 
 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 ,


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 = 
 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 ,
___
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

[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

[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

[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

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

[Derek Schuff via Phabricator via cfe-commits]

dschuff updated this revision to Diff 292056.
dschuff added a comment.
Herald added subscribers: sstefan1, ormris, jgravelle-google.
Herald added a reviewer: jdoerfert.

upload the correct diff


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_GNU_str_index
+; CHECK: DW_AT_GNU_dwo_name  DW_FORM_GNU_str_index
+; CHECK-NOT: DW_AT_low_pc
+; CHECK-NOT: 

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

[Derek Schuff via Phabricator via cfe-commits]

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

Get the right sections in the objects, add tests


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
@@ -321,9 +321,8 @@
 
   void executePostLayoutBinding(MCAssembler ,
 const MCAsmLayout ) override;
-  void prepareImports(SmallVectorImpl& Imports,
-  MCAssembler ,
-  const MCAsmLayout );
+  void prepareImports(SmallVectorImpl ,
+  MCAssembler , const MCAsmLayout );
   uint64_t writeObject(MCAssembler , const MCAsmLayout ) override;
 
   uint64_t writeOneObject(MCAssembler , const MCAsmLayout ,
@@ -963,7 +962,7 @@
   encodeULEB128(0, W->OS); // memory index
 if ((Segment.InitFlags & wasm::WASM_SEGMENT_IS_PASSIVE) == 0) {
   W->OS << char(Segment.Offset > INT32_MAX ? wasm::WASM_OPCODE_I64_CONST
-  : wasm::WASM_OPCODE_I32_CONST);
+   : wasm::WASM_OPCODE_I32_CONST);
   encodeSLEB128(Segment.Offset, W->OS); // offset
   W->OS << char(wasm::WASM_OPCODE_END);
 }
@@ -1198,9 +1197,9 @@
 
   return true;
 }
-void WasmObjectWriter::prepareImports(SmallVectorImpl& 
Imports,
-  MCAssembler ,
-  const MCAsmLayout ) {
+void WasmObjectWriter::prepareImports(
+SmallVectorImpl , MCAssembler ,
+const MCAsmLayout ) {
   // For now, always emit the memory import, since loads and stores are not
   // valid without it. In the future, we could perhaps be more clever and omit
   // it if there are no loads or stores.


Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -321,9 +321,8 @@
 
   void executePostLayoutBinding(MCAssembler ,
 const MCAsmLayout ) override;
-  void prepareImports(SmallVectorImpl& Imports,
-  MCAssembler ,
-  const MCAsmLayout );
+  void prepareImports(SmallVectorImpl ,
+  MCAssembler , const MCAsmLayout );
   uint64_t writeObject(MCAssembler , const MCAsmLayout ) override;
 
   uint64_t writeOneObject(MCAssembler , const MCAsmLayout ,
@@ -963,7 +962,7 @@
   encodeULEB128(0, W->OS); // memory index
 if ((Segment.InitFlags & wasm::WASM_SEGMENT_IS_PASSIVE) == 0) {
   W->OS << char(Segment.Offset > INT32_MAX ? wasm::WASM_OPCODE_I64_CONST
-  : wasm::WASM_OPCODE_I32_CONST);
+   : wasm::WASM_OPCODE_I32_CONST);
   encodeSLEB128(Segment.Offset, W->OS); // offset
   W->OS << char(wasm::WASM_OPCODE_END);
 }
@@ -1198,9 +1197,9 @@
 
   return true;
 }
-void WasmObjectWriter::prepareImports(SmallVectorImpl& Imports,
-  MCAssembler ,
-  const MCAsmLayout ) {
+void WasmObjectWriter::prepareImports(
+SmallVectorImpl , MCAssembler ,
+const MCAsmLayout ) {
   // For now, always emit the memory import, since loads and stores are not
   // valid without it. In the future, we could perhaps be more clever and omit
   // it if there are no loads or stores.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Derek Schuff via Phabricator via cfe-commits]

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

Fix bad diff


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
  llvm/include/llvm/MC/MCWasmObjectWriter.h
  llvm/lib/MC/MCAsmBackend.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
@@ -216,8 +216,12 @@
   Stream.pwrite((char *)Buffer, sizeof(Buffer), Offset);
 }
 
+bool isDwoSection(const MCSection ) {
+  return Sec.getName().endswith(".dwo");
+}
+
 class WasmObjectWriter : public MCObjectWriter {
-  support::endian::Writer W;
+  support::endian::Writer *W;
 
   /// The target specific Wasm writer instance.
   std::unique_ptr TargetObjectWriter;
@@ -260,7 +264,16 @@
   unsigned NumEventImports = 0;
   uint32_t SectionCount = 0;
 
-  // TargetObjectWriter wrappers.
+  enum class DwoMode {
+AllSections,
+NonDwoOnly,
+DwoOnly,
+  };
+  bool IsSplitDwarf = false;
+  raw_pwrite_stream *OS = nullptr;
+  raw_pwrite_stream *DwoOS = nullptr;
+
+  // TargetObjectWriter wranppers.
   bool is64Bit() const { return TargetObjectWriter->is64Bit(); }
   bool isEmscripten() const { return TargetObjectWriter->isEmscripten(); }
 
@@ -270,8 +283,12 @@
 
 public:
   WasmObjectWriter(std::unique_ptr MOTW,
-   raw_pwrite_stream )
-  : W(OS, support::little), TargetObjectWriter(std::move(MOTW)) {}
+   raw_pwrite_stream _)
+  : TargetObjectWriter(std::move(MOTW)), OS(_) {}
+  WasmObjectWriter(std::unique_ptr MOTW,
+   raw_pwrite_stream _, raw_pwrite_stream _)
+  : TargetObjectWriter(std::move(MOTW)), IsSplitDwarf(true), OS(_),
+DwoOS(_) {}
 
 private:
   void reset() override {
@@ -306,24 +323,27 @@
 
   uint64_t writeObject(MCAssembler , const MCAsmLayout ) override;
 
+  uint64_t writeOneObject(MCAssembler , const MCAsmLayout ,
+  DwoMode Mode);
+
   void writeString(const StringRef Str) {
-encodeULEB128(Str.size(), W.OS);
-W.OS << Str;
+encodeULEB128(Str.size(), W->OS);
+W->OS << Str;
   }
 
   void writeI32(int32_t val) {
 char Buffer[4];
 support::endian::write32le(Buffer, val);
-W.OS.write(Buffer, sizeof(Buffer));
+W->OS.write(Buffer, sizeof(Buffer));
   }
 
   void writeI64(int64_t val) {
 char Buffer[8];
 support::endian::write64le(Buffer, val);
-W.OS.write(Buffer, sizeof(Buffer));
+W->OS.write(Buffer, sizeof(Buffer));
   }
 
-  void writeValueType(wasm::ValType Ty) { W.OS << static_cast(Ty); }
+  void writeValueType(wasm::ValType Ty) { W->OS << static_cast(Ty); }
 
   void writeTypeSection(ArrayRef Signatures);
   void writeImportSection(ArrayRef Imports, uint64_t DataSize,
@@ -368,17 +388,17 @@
 void WasmObjectWriter::startSection(SectionBookkeeping ,
 unsigned SectionId) {
   LLVM_DEBUG(dbgs() << "startSection " << SectionId << "\n");
-  W.OS << char(SectionId);
+  W->OS << char(SectionId);
 
-  Section.SizeOffset = W.OS.tell();
+  Section.SizeOffset = W->OS.tell();
 
   // The section size. We don't know the size yet, so reserve enough space
   // for any 32-bit value; we'll patch it later.
-  encodeULEB128(0, W.OS, 5);
+  encodeULEB128(0, W->OS, 5);
 
   // The position where the section starts, for measuring its size.
-  Section.ContentsOffset = W.OS.tell();
-  Section.PayloadOffset = W.OS.tell();
+  Section.ContentsOffset = W->OS.tell();
+  Section.PayloadOffset = W->OS.tell();
   Section.Index = SectionCount++;
 }
 
@@ -388,19 +408,19 @@
   startSection(Section, wasm::WASM_SEC_CUSTOM);
 
   // The position where the section header ends, for measuring its size.
-  Section.PayloadOffset = W.OS.tell();
+  Section.PayloadOffset = W->OS.tell();
 
   // Custom sections in wasm also have a string identifier.
   writeString(Name);
 
   // The position where the custom section starts.
-  Section.ContentsOffset = W.OS.tell();
+  Section.ContentsOffset = W->OS.tell();
 }
 
 // Now that the section is complete and we know how big it is, patch up the
 // section size field at the start of the section.
 void WasmObjectWriter::endSection(SectionBookkeeping ) {
-  uint64_t Size = W.OS.tell();
+  uint64_t Size = W->OS.tell();
   // /dev/null doesn't support seek/tell and can report offset of 0.
   // Simply skip this patching in that case.
   if (!Size)
@@ -414,14 +434,14 @@
 
   // Write the final section size to the payload_len field, which follows
   // the section id byte.
-  writePatchableLEB<5>(static_cast(W.OS), Size,
+  writePatchableLEB<5>(static_cast(W->OS), Size,
Section.SizeOffset);
 }
 
 // Emit the Wasm header.
 void WasmObjectWriter::writeHeader(const MCAssembler ) {
-  W.OS.write(wasm::WasmMagic, 

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

[Derek Schuff via Phabricator via cfe-commits]

dschuff created this revision.
dschuff added reviewers: sbc100, aardappel.
Herald added subscribers: llvm-commits, cfe-commits, sunfish, hiraditya, 
aprantl.
Herald added projects: clang, LLVM.
dschuff requested review of this revision.
Herald added a subscriber: aheejin.

Initial support for dwarf fission sections (-gsplit-dwarf) on wasm.
The most interesting change is support for writing 2 files (.o and .dwo) in the
wasm object writer. My approach moves object-writing logic into its own function
and calls it twice, swapping out the endian::Writer (W) in between calls.
(This is a bit different than the ELF writer's approach. I like it a bit better
but don't have a strong opinion).

This patch has the basic structure and writes separate files containing
non-dwo and dwo sections. There are couple of other things needed:

1. Linker support for an equivalent of ELF's SHF_EXCLUDE to keep the debug 
sections from being linked
2. a few checks and validations (equivalent to the places where the ELF object 
writer checks the dwo mode, such as validating relocs)

These can go in future CLs, but I still need to add some tests to this one.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85685

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/lib/MC/MCAsmBackend.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
@@ -270,8 +270,8 @@
 DwoOnly,
   };
   bool IsSplitDwarf = false;
-  raw_pwrite_stream* OS = nullptr;
-  raw_pwrite_stream* DwoOS = nullptr;
+  raw_pwrite_stream *OS = nullptr;
+  raw_pwrite_stream *DwoOS = nullptr;
 
   // TargetObjectWriter wranppers.
   bool is64Bit() const { return TargetObjectWriter->is64Bit(); }
@@ -287,9 +287,8 @@
   : TargetObjectWriter(std::move(MOTW)), OS(_) {}
   WasmObjectWriter(std::unique_ptr MOTW,
raw_pwrite_stream _, raw_pwrite_stream _)
-  : TargetObjectWriter(std::move(MOTW)),
-IsSplitDwarf(true), OS(_), DwoOS(_) {}
-
+  : TargetObjectWriter(std::move(MOTW)), IsSplitDwarf(true), OS(_),
+DwoOS(_) {}
 
 private:
   void reset() override {
@@ -324,8 +323,8 @@
 
   uint64_t writeObject(MCAssembler , const MCAsmLayout ) override;
 
-  uint64_t writeOneObject(MCAssembler , const MCAsmLayout , DwoMode Mode);
-
+  uint64_t writeOneObject(MCAssembler , const MCAsmLayout ,
+  DwoMode Mode);
 
   void writeString(const StringRef Str) {
 encodeULEB128(Str.size(), W->OS);
@@ -781,7 +780,7 @@
   break;
 case wasm::WASM_EXTERNAL_MEMORY:
   encodeULEB128(Import.Memory.Flags, W->OS);
-  encodeULEB128(NumPages, W->OS);  // initial
+  encodeULEB128(NumPages, W->OS); // initial
   break;
 case wasm::WASM_EXTERNAL_TABLE:
   W->OS << char(Import.Table.ElemType);
@@ -961,8 +960,8 @@
   encodeULEB128(0, W->OS); // memory index
 if ((Segment.InitFlags & wasm::WASM_SEGMENT_IS_PASSIVE) == 0) {
   W->OS << char(Segment.Offset > std::numeric_limits().max()
- ? wasm::WASM_OPCODE_I64_CONST
- : wasm::WASM_OPCODE_I32_CONST);
+? wasm::WASM_OPCODE_I64_CONST
+: wasm::WASM_OPCODE_I32_CONST);
   encodeSLEB128(Segment.Offset, W->OS); // offset
   W->OS << char(wasm::WASM_OPCODE_END);
 }
@@ -1771,6 +1770,7 @@
 
 std::unique_ptr
 llvm::createWasmDwoObjectWriter(std::unique_ptr MOTW,
-raw_pwrite_stream , raw_pwrite_stream ) {
+raw_pwrite_stream ,
+raw_pwrite_stream ) {
   return std::make_unique(std::move(MOTW), OS);
 }
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -796,8 +796,10 @@
   DwarfFrameSection = Ctx->getWasmSection(".debug_frame", SectionKind::getMetadata());
   DwarfPubNamesSection = Ctx->getWasmSection(".debug_pubnames", SectionKind::getMetadata());
   DwarfPubTypesSection = Ctx->getWasmSection(".debug_pubtypes", SectionKind::getMetadata());
-  DwarfGnuPubNamesSection = Ctx->getWasmSection(".debug_gnu_pubnames", SectionKind::getMetadata());
-  DwarfGnuPubTypesSection = Ctx->getWasmSection(".debug_gnu_pubtypes", SectionKind::getMetadata());
+  DwarfGnuPubNamesSection =
+  Ctx->getWasmSection(".debug_gnu_pubnames", SectionKind::getMetadata());
+  DwarfGnuPubTypesSection =
+  Ctx->getWasmSection(".debug_gnu_pubtypes", SectionKind::getMetadata());
 
   DwarfDebugNamesSection =
   Ctx->getWasmSection(".debug_names", SectionKind::getMetadata());
@@ -817,15 +819,15 @@
   Ctx->getWasmSection(".debug_types.dwo", SectionKind::getMetadata());
   DwarfAbbrevDWOSection =
   Ctx->getWasmSection(".debug_abbrev.dwo", 

[PATCH] D82130: [WebAssembly] Adding 64-bit versions of __stack_pointer and other globals

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.

Compiler changes LGTM




Comment at: llvm/test/CodeGen/WebAssembly/stack-alignment.ll:1
-; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt 
-wasm-keep-registers | FileCheck %s
-
-target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-target triple = "wasm32-unknown-unknown"
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false 
-disable-wasm-fallthrough-return-opt -wasm-keep-registers | FileCheck -DPTR=32 
%s
+; RUN: llc < %s --mtriple=wasm64-unknown-unknown -asm-verbose=false 
-disable-wasm-fallthrough-return-opt -wasm-keep-registers | FileCheck -DPTR=64 
%s

that -D flag is really nice.


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

https://reviews.llvm.org/D82130



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

[PATCH] D82130: [WebAssembly] Adding 64-bit versions of __stack_pointer and other globals

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

So the code LGTM, were you going to add to usertest.ll in this CL?




Comment at: llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp:84
 WasmSym->setGlobalType(wasm::WasmGlobalType{
-uint8_t(Subtarget.hasAddr64() ? wasm::WASM_TYPE_I64
-  : wasm::WASM_TYPE_I32),
+uint8_t(Subtarget.hasAddr64() && strcmp(Name, "__table_base") != 0
+? wasm::WASM_TYPE_I64

aardappel wrote:
> dschuff wrote:
> > should __table_base stay as i32?
> I'd think so, right? since it refers to table indices, not memory
Oh, right; I'd misread this as setting it just for table_base but I had it 
backwards; it's exempting table_base. So yeah this is right.


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

https://reviews.llvm.org/D82130



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

[PATCH] D82130: [WebAssembly] Adding 64-bit versions of __stack_pointer and other globals

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Yeah I think a 64-bit version of userstack.ll makes sense.  (a fork or a second 
set of CHECKs, whatever you think would be cleaner). There's also 
`stack-alignment.ll` which covers dynamic stack adjustment.




Comment at: lld/wasm/Driver.cpp:385
+StringRef s = arg->getValue();
+if (s == "wasm32")
+  config->is64 = false;

any particular reason this shouldn't use the more conventional `-m32`/`m64`?
edit: nevermind, this is lld not clang. I'll defer to Sam's opinion on lld 
flags.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp:84
 WasmSym->setGlobalType(wasm::WasmGlobalType{
-uint8_t(Subtarget.hasAddr64() ? wasm::WASM_TYPE_I64
-  : wasm::WASM_TYPE_I32),
+uint8_t(Subtarget.hasAddr64() && strcmp(Name, "__table_base") != 0
+? wasm::WASM_TYPE_I64

should __table_base stay as i32?


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

https://reviews.llvm.org/D82130



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

[PATCH] D80362: [WebAssembly] Warn on exception spec only when Wasm EH is used

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

otherwise LGTM




Comment at: clang/docs/DiagnosticsReference.rst:14018
++---+
+|:warning:`warning:` |nbsp| :diagtext:`dynamic exception specifications with 
types are currently ignored in wasm exception handling`|
++---+

I think it's not actually necessary to change the text of the warning. Since 
the warning is about an exception handling language feature, I think adding 
"exception handling" on the end doesn't make it any more clear and sounds a 
little redundant to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80362



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

[PATCH] D79655: [WebAssembly] Handle exception specifications

[Derek Schuff via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGenCXX/wasm-eh.cpp:399
 
+// RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions 
-fexceptions -fcxx-exceptions -fwasm-exceptions -target-feature 
+exception-handling -S -o - -std=c++11 | FileCheck %s --check-prefix=ASSEMBLY
+

aheejin wrote:
> This was preexisting just moved
Is it common in these tests to have RUN lines throughout the file instead of 
all together up at the top?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79655



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

[PATCH] D79655: [WebAssembly] Ignore exception specifications

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Actually, would it be possible to not ignore `throw()` but make it an alias for 
`noexcept(true)`? Apparently that is the standard behavior in C++17, so it 
might make more sense to just implement that now rather than just warning all 
the time and ignoring it. It would also cover most of the cases that exist, so 
more users wouldn't need to disable the warning.




Comment at: clang/lib/CodeGen/CGException.cpp:23
 #include "clang/AST/StmtVisitor.h"
+#include "clang/Basic/DiagnosticSema.h"
 #include "clang/Basic/TargetBuiltins.h"

aheejin wrote:
> aheejin wrote:
> > One thing I'm not sure about is if it is a good thing to include this in 
> > CodeGen. Usually this header is used in Sema/ directory. This depends on 
> > which .td file I add `warn_wasm_exception_spec_ignored`. 
> > DiagnosticSemaKinds.td seemed to have a section in exception spec so I 
> > added it there, but not entirely sure if that's the best location to add it.
> Oh, and this warning message was requested by Adobe, but I think it's good to 
> have in general.
According to the CMakeLists.txt in lib/CodeGen,  clangCodeGen depends on 
clangBasic, so I think it should be ok to include stuff from clang/Basic here 
(even if it has Sema in the name).
And a warning seems ok, as long as it's possible to suppress it. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79655



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

We can definitely still talk about what you are trying to do, and it would 
probably be useful to have more folks involved. Opening an issue on 
https://github.com/WebAssembly/tool-conventions/ might be useful since it 
involves the conventions that LLVM and clang use. If it's specific to the web, 
then https://groups.google.com/forum/#!forum/emscripten-discuss could be 
another place (even if you don't plan on using emscripten's JS code).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D78441: Delete NaCl support

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

@jfb thanks for the heads-up.
I replied on the mailing list thread.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78441



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

[PATCH] D71823: Support Swift calling convention for WebAssembly targets

[Derek Schuff via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc5bd3d07262f: Support Swift calling convention for 
WebAssembly targets (authored by kateinoigakukun, committed by dschuff).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71823

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp


Index: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -672,7 +672,8 @@
  CallConv == CallingConv::PreserveMost ||
  CallConv == CallingConv::PreserveAll ||
  CallConv == CallingConv::CXX_FAST_TLS ||
- CallConv == CallingConv::WASM_EmscriptenInvoke;
+ CallConv == CallingConv::WASM_EmscriptenInvoke ||
+ CallConv == CallingConv::Swift;
 }
 
 SDValue
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -114,6 +114,16 @@
? (IsSigned ? SignedLongLong : UnsignedLongLong)
: TargetInfo::getLeastIntTypeByWidth(BitWidth, IsSigned);
   }
+
+  CallingConvCheckResult checkCallingConvention(CallingConv CC) const override 
{
+switch (CC) {
+case CC_C:
+case CC_Swift:
+  return CCCR_OK;
+default:
+  return CCCR_Warning;
+}
+  }
 };
 class LLVM_LIBRARY_VISIBILITY WebAssembly32TargetInfo
 : public WebAssemblyTargetInfo {


Index: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -672,7 +672,8 @@
  CallConv == CallingConv::PreserveMost ||
  CallConv == CallingConv::PreserveAll ||
  CallConv == CallingConv::CXX_FAST_TLS ||
- CallConv == CallingConv::WASM_EmscriptenInvoke;
+ CallConv == CallingConv::WASM_EmscriptenInvoke ||
+ CallConv == CallingConv::Swift;
 }
 
 SDValue
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -114,6 +114,16 @@
? (IsSigned ? SignedLongLong : UnsignedLongLong)
: TargetInfo::getLeastIntTypeByWidth(BitWidth, IsSigned);
   }
+
+  CallingConvCheckResult checkCallingConvention(CallingConv CC) const override {
+switch (CC) {
+case CC_C:
+case CC_Swift:
+  return CCCR_OK;
+default:
+  return CCCR_Warning;
+}
+  }
 };
 class LLVM_LIBRARY_VISIBILITY WebAssembly32TargetInfo
 : public WebAssemblyTargetInfo {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71823: Support Swift calling convention for WebAssembly targets

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

Sorry we left this dangling for so long. This little change looks fine; I guess 
you must be in the process of porting swift to wasm? Do you have any more 
detailed plans for that written down anywhere, or a prototype you can show?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71823



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

[PATCH] D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: lld/test/wasm/export-name.ll:20
+; CHECK-NEXT:Exports:
+; CHECK-NEXT:  - Name:memory
+; CHECK-NEXT:Kind:MEMORY

sbc100 wrote:
> dschuff wrote:
> > does this test need to verify that the memory and _start are exported? 
> > seems like just a check for bar would be enough.
> The worry that if I just look for ` Name:bar` it might appear in 
> some other section too mabye?
You could do `CHECK-LABEL: Exports` and then `CHECK: - Name: bar` and then 
`CHECK-LABEL` on the next section?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70520



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

[PATCH] D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names

[Derek Schuff via Phabricator via cfe-commits]

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



Comment at: lld/test/wasm/export-name.ll:20
+; CHECK-NEXT:Exports:
+; CHECK-NEXT:  - Name:memory
+; CHECK-NEXT:Kind:MEMORY

does this test need to verify that the memory and _start are exported? seems 
like just a check for bar would be enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70520



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

I do find it odd that there is a PATH fallback in the existing code in the 
first place. I agree that basically no compiler other than the "system" 
compiler should ever use it (and also even the concept of the "system" compiler 
really only makes much sense on systems like Linux and BSDs where compiling 
things for the local system is common). I guess the other option here would be 
to just require that wasm-opt be in the same directory as clang, which we can 
arrange in wasi-sdk or wherever.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70877: [WebAssebmly][MC] Support .import_name/.import_field asm directives

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/test/MC/WebAssembly/import-module.s:13
+  .import_module  foo, bar
+  .import_name  foo, qux
+

What should happen if there's a directive that refers to a symbol that doesn't 
exist in the asm file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70877



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Also if at some point we are able to remove a bunch of the driver logic from 
emcc and move it here, (e.g. running clang to link instead of lld directly) 
we'll need to watch out for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

WRT the LTO directory name, there's theoretically the danger that someone (e.g. 
emscripten) could be doing a rolling release of the compiler and get 
invalidated within a major revision. But in that case, 1) they should be 
rebuilding their libraries on each release anyway, and 2) last time I checked, 
policy was to make auto-upgrade of bitcode work within a revision. So it's 
probably not a problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

LGTM on the approach, just one more question on the wasm-opt flags.




Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:105
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();

This chain is slightly confusing. I assume this `getValue()` captures the 
number in `-O3`, `-O2`, etc? But why then do we need special-casing for 0 and 4 
above?

For that matter, we should probably not run wasm-opt at all if the opt-level is 
0, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

sunfish wrote:
> dschuff wrote:
> > sunfish wrote:
> > > dschuff wrote:
> > > > sunfish wrote:
> > > > > dschuff wrote:
> > > > > > What would you think about adding a way to pass arguments through 
> > > > > > to wasm-opt on the command line, like we have for the linker, 
> > > > > > assembler, etc? Something like `-Wo,-O2` (or `-Ww` or whatever; 
> > > > > > analogous to `-Wl` and `-Wa`). That seems nicer than an env var, 
> > > > > > although it doesn't solve the problem of controlling whether to run 
> > > > > > the optimizer in the first place.
> > > > > My guess here is that we don't need clang to have an option for 
> > > > > passing additional flags -- if people want to do extra special things 
> > > > > with wasm-opt on clang's output they can just run wasm-opt directly 
> > > > > themselves. Does that sound reasonable?
> > > > Maybe. But I still don't like the use of an env var for this kind of 
> > > > behavior-effecting (i.e. non-debugging) use case.  It's hard enough to 
> > > > get reproducible and hermetic build behavior as it is, I definitely 
> > > > wouldn't want to worry about the environment affecting the output in 
> > > > addition to all the random flags, included files, etc.
> > > If we did -Wo,-O2 or so, we'd still need to be able to find the wasm-opt 
> > > program to be able to run it. We could just search for it in PATH, but 
> > > that's also a little dicey from a hermetic build perspective.
> > > 
> > > I borrowed "WASM_OPT" from 
> > > [cargo-wasi](https://github.com/bytecodealliance/cargo-wasi). I'm also 
> > > not a fan of environment variables in general, but this way does have the 
> > > advantage that people can set it once, and not have to modify their 
> > > Makefiles to add new flags. Users can think of it as just being part of 
> > > -O2 and friends.
> > > 
> > What's the usual way to locate things like external assemblers? Presumably 
> > we could use the same mechanism for wasm-opt?
> It checks the COMPILER_PATH environment variable and -B command-line flags, 
> which I'm not sure we should use here, but it looks like it does fall back to 
> checking PATH at the end.
> 
> So, what would you think of just checking PATH for wasm-opt?
I suspect we'll end up with -B flags if/when people start building interesting 
or nontrivial toolchains with clang (or we try to make emscripten more 
standardish), but I'm fine with leaving that out for now. Checking PATH for 
wasm-opt seems fine to me to locate the binary. Did you have that in mind as 
also the way to determine whether or not to run wasm-opt (i.e. run if it's 
there, don't if it's not)? That seems slightly error-prone in the sense that 
there would be a silent behavior change if something went wrong (e.g. wasm-opt 
goes missing) but in a world where most clang users are using wasm-opt then 
using wasm-opt by default seems reasonable; so that seems fine as a way to 
start out.

I do think we will eventually want some way to modify the behavior of wasm-opt 
though. For that matter wasm-opt might at some point become able to optimize 
object files (allowing faster links at the cost of less LTO-optimized binaries; 
we'd run a reduced set of passes post-link or none at all). In that case 
there'd also have to be further changes if we want builtin support for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

sunfish wrote:
> dschuff wrote:
> > sunfish wrote:
> > > dschuff wrote:
> > > > What would you think about adding a way to pass arguments through to 
> > > > wasm-opt on the command line, like we have for the linker, assembler, 
> > > > etc? Something like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl` 
> > > > and `-Wa`). That seems nicer than an env var, although it doesn't solve 
> > > > the problem of controlling whether to run the optimizer in the first 
> > > > place.
> > > My guess here is that we don't need clang to have an option for passing 
> > > additional flags -- if people want to do extra special things with 
> > > wasm-opt on clang's output they can just run wasm-opt directly 
> > > themselves. Does that sound reasonable?
> > Maybe. But I still don't like the use of an env var for this kind of 
> > behavior-effecting (i.e. non-debugging) use case.  It's hard enough to get 
> > reproducible and hermetic build behavior as it is, I definitely wouldn't 
> > want to worry about the environment affecting the output in addition to all 
> > the random flags, included files, etc.
> If we did -Wo,-O2 or so, we'd still need to be able to find the wasm-opt 
> program to be able to run it. We could just search for it in PATH, but that's 
> also a little dicey from a hermetic build perspective.
> 
> I borrowed "WASM_OPT" from 
> [cargo-wasi](https://github.com/bytecodealliance/cargo-wasi). I'm also not a 
> fan of environment variables in general, but this way does have the advantage 
> that people can set it once, and not have to modify their Makefiles to add 
> new flags. Users can think of it as just being part of -O2 and friends.
> 
What's the usual way to locate things like external assemblers? Presumably we 
could use the same mechanism for wasm-opt?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

If the SDK is distributed with the compiler and version-locked, it seems like 
it should be ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

sunfish wrote:
> dschuff wrote:
> > What would you think about adding a way to pass arguments through to 
> > wasm-opt on the command line, like we have for the linker, assembler, etc? 
> > Something like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl` and 
> > `-Wa`). That seems nicer than an env var, although it doesn't solve the 
> > problem of controlling whether to run the optimizer in the first place.
> My guess here is that we don't need clang to have an option for passing 
> additional flags -- if people want to do extra special things with wasm-opt 
> on clang's output they can just run wasm-opt directly themselves. Does that 
> sound reasonable?
Maybe. But I still don't like the use of an env var for this kind of 
behavior-effecting (i.e. non-debugging) use case.  It's hard enough to get 
reproducible and hermetic build behavior as it is, I definitely wouldn't want 
to worry about the environment affecting the output in addition to all the 
random flags, included files, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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