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

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 e

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

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

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

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. stdd

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

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 `clan

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

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 `WebAsse

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

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/D159

[PATCH] D78441: Delete NaCl support

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/Nmz

[PATCH] D78441: Delete NaCl support

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 Monore

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

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:

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

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

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

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

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

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

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

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 chan

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

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 i

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

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 ou

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

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 an

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

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 cause

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

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 project

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

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

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

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.llv

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

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 ___

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

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.l

[PATCH] D78441: Delete NaCl support

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.o

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

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.ll

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

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 __

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

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 passin

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

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 emscr

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

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/ http

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

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://

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

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

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

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/Dr

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

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 AFA

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

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 no

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

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 ex

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

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-emscript

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

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 att

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

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 LowerEmscrip

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

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

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

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 th

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

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 L

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

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:

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

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/Targ

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

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 Git

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

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 d1a96e906

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

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.o

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

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

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

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:

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

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 curiosit

[PATCH] D100981: Delete le32/le64 targets

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

[PATCH] D100981: Delete le32/le64 targets

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"

[PATCH] D99623: [WebAssembly] Implement i64x2 comparisons

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 t

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

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(__builti

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

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/cla

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

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 CHAN

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

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

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

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 Ub

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

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 arc

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

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

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

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

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

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:

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

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/debu

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

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

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

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.llv

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

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

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

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: l

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

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: > > dblaiki

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

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

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

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 tes

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

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

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

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.

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

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

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

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/

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

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 designe

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

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 spl

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

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

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

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/

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

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.

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

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/MCWa

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

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

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

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-

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

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 -

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

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 +Stri

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

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 ++-

[PATCH] D79655: [WebAssembly] Handle exception specifications

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-exceptio

[PATCH] D79655: [WebAssembly] Ignore exception specifications

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 al

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

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

[PATCH] D78441: Delete NaCl support

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 mai

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

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://revie

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

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

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

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

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

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 -

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

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 lik

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

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: r

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

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

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

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

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

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();

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

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

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

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

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

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 ___

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

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

  1   2   >