Page MenuHomePhabricator

sunfish (Dan Gohman)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 12 2013, 11:44 AM (450 w, 6 d)

Recent Activity

Wed, Jun 29

sunfish added a comment to D81689: [WebAssembly] New-style command support.

there are use cases for a command to export non-main functions.
eg. malloc/free stuff mentioned in https://github.com/bytecodealliance/wasm-micro-runtime/blob/main/doc/memory_tune.md
this change broke them.

One consideration is that that wamr API doesn't seem compatible with closest thing we have to a spec, which states "Command instances may assume that they will be called from the environment at most once. Command instances may assume that none of their exports are accessed outside the duration of that call.". It's debatable how authoritative that spec is, but the big picture is that there's not currently much clarity on the relationship between LLVM and engines.

  • if "outside the duration of that call" mean it's ok to access exports during the call of _start, the wamr api seems compatible to me. (those malloc/free exports are used during the execution of _start, as far as i understand.)
Wed, Jun 29, 8:59 AM · Restricted Project, Restricted Project

Mon, Jun 27

sunfish added a comment to D81689: [WebAssembly] New-style command support.

there are use cases for a command to export non-main functions.
eg. malloc/free stuff mentioned in https://github.com/bytecodealliance/wasm-micro-runtime/blob/main/doc/memory_tune.md
this change broke them.

Mon, Jun 27, 1:50 PM · Restricted Project, Restricted Project

Wed, Jun 15

sunfish added reviewers for D125728: [WebAssembly] Update supported features in -mcpu=generic: asb, sbc100.
Wed, Jun 15, 11:48 AM · Restricted Project, Restricted Project
sunfish added reviewers for D125729: [WebAssembly] Update supported features in the generic CPU configuration: asb, sbc100.
Wed, Jun 15, 11:47 AM · Restricted Project, Restricted Project
sunfish accepted D127888: [clang][WebAssembly] Loosen restriction on `main` symbol mangling.
Wed, Jun 15, 11:46 AM · Restricted Project, Restricted Project

Jun 1 2022

sunfish accepted D75277: [WebAssembly] Remove restriction on main name mangling.
Jun 1 2022, 9:29 AM · Restricted Project, Restricted Project, Restricted Project

May 31 2022

sunfish added inline comments to D75277: [WebAssembly] Remove restriction on main name mangling.
May 31 2022, 3:28 PM · Restricted Project, Restricted Project, Restricted Project
sunfish added inline comments to D75277: [WebAssembly] Remove restriction on main name mangling.
May 31 2022, 3:02 PM · Restricted Project, Restricted Project, Restricted Project
sunfish added inline comments to D75277: [WebAssembly] Remove restriction on main name mangling.
May 31 2022, 1:46 PM · Restricted Project, Restricted Project, Restricted Project

May 20 2022

sunfish updated the diff for D125729: [WebAssembly] Update supported features in the generic CPU configuration.

Update tests to use -mcpu=mvp to avoid breaking their target features when -mcpu=generic changes. And update target-features.ll to for the new -mcpu=generic features.

May 20 2022, 3:55 PM · Restricted Project, Restricted Project
sunfish committed rG59726668f1dc: [WebAssembly] Strip TLS when "atomics" is not enabled (authored by sunfish).
[WebAssembly] Strip TLS when "atomics" is not enabled
May 20 2022, 3:20 PM · Restricted Project, Restricted Project
sunfish closed D125730: [WebAssembly] Strip TLS when "atomics" is not enabled.
May 20 2022, 3:20 PM · Restricted Project, Restricted Project
sunfish updated the diff for D125728: [WebAssembly] Update supported features in -mcpu=generic.

Add tests for -mcpu=mvp and -mcpu=bleeding-edge.

May 20 2022, 2:52 PM · Restricted Project, Restricted Project
sunfish updated the diff for D125728: [WebAssembly] Update supported features in -mcpu=generic.

Add a driver test, and add release notes.

May 20 2022, 2:46 PM · Restricted Project, Restricted Project

May 16 2022

sunfish requested review of D125730: [WebAssembly] Strip TLS when "atomics" is not enabled.
May 16 2022, 3:36 PM · Restricted Project, Restricted Project
sunfish requested review of D125729: [WebAssembly] Update supported features in the generic CPU configuration.
May 16 2022, 2:56 PM · Restricted Project, Restricted Project
sunfish requested review of D125728: [WebAssembly] Update supported features in -mcpu=generic.
May 16 2022, 2:49 PM · Restricted Project, Restricted Project

Mar 14 2022

sunfish added a comment to D121327: Lower `@llvm.global_dtors` using `__cxa_atexit` on MachO.

@sunfish
Hi Dan, I hope you are still happy with this change. I didn't change any WebAssembly tests, but rather added a new IR-level test, so all existing WebAssembly behavior should stay the same. Let me know if you have any concerns.

Mar 14 2022, 5:47 PM · Restricted Project, Restricted Project, Restricted Project

Mar 9 2022

sunfish added a comment to D121327: Lower `@llvm.global_dtors` using `__cxa_atexit` on MachO.

The overall approach here makes sense to me. The tests test/CodeGen/WebAssembly/global_dtors.ll and test/CodeGen/WebAssembly/lower-global-dtors.ll cover the main interesting corner cases that we've hit with this pass; they currently have wasm targets and wasm-specific CHECK lines, but could likely be ported to other targets.

Mar 9 2022, 1:36 PM · Restricted Project, Restricted Project, Restricted Project

Jul 21 2021

sunfish added a comment to D106499: [lld][WebAssembly] Align __heap_base.

Looks good to me too!

Jul 21 2021, 3:53 PM · Restricted Project

Jul 9 2021

sunfish added inline comments to D105749: WebAssembly: Update datalayout to match fp128 ABI change.
Jul 9 2021, 6:35 PM · Restricted Project, Restricted Project

Jun 23 2021

sunfish added a comment to D104808: [clang][emscripten] Reduce alignof long double from 16 to 8 bytes.

Do we still intend to unify Emscripten's ABI with wasm32-unknown-unknown or wasm32-wasi eventually? This is talking a step away from that.

Jun 23 2021, 1:02 PM · Restricted Project

May 6 2021

sunfish added inline comments to D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR.
May 6 2021, 5:38 PM · Restricted Project, Restricted Project
sunfish added inline comments to D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR.
May 6 2021, 3:17 PM · Restricted Project, Restricted Project
sunfish added inline comments to D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR.
May 6 2021, 11:23 AM · Restricted Project, Restricted Project
sunfish added inline comments to D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR.
May 6 2021, 10:43 AM · Restricted Project, Restricted Project

Apr 28 2021

sunfish added a comment to D101271: [lld][WebAssembly] Add support for `--export-dynamic-symbol`.

So you think it would be preferable to go with --export-if-defined then, or something wasm specific?

Apr 28 2021, 3:43 PM · Restricted Project
sunfish added a comment to D101271: [lld][WebAssembly] Add support for `--export-dynamic-symbol`.

Looking at the ld documentation for export-dynamic-symbol:

Apr 28 2021, 11:59 AM · Restricted Project
sunfish added a comment to D101271: [lld][WebAssembly] Add support for `--export-dynamic-symbol`.

I'm a little confused by the name "export-dynamic-symbol". I understand it's an ELF flag, though it seems to have a slightly different meaning here. What would you think about adding a warning when using this flag is -experimental-pic isn't passed, similar to what we do with -shared?

Apr 28 2021, 10:51 AM · Restricted Project

Apr 13 2021

sunfish added a comment to D100411: [WebAssembly] Use standard intrinsics for f32x4 and f64x2 ops.

Great, thanks! And yes, switching to roundeven for both scalar and SIMD ISel sounds right to me.

Apr 13 2021, 5:03 PM · Restricted Project, Restricted Project
sunfish accepted D100411: [WebAssembly] Use standard intrinsics for f32x4 and f64x2 ops.

This looks good to me! Could you briefly comment here on what the issue with llvm.roundeven is?

Apr 13 2021, 4:15 PM · Restricted Project, Restricted Project

Feb 26 2021

sunfish committed rGc62dabc3f501: [WebAssembly] Avoid `bit_cast` when printing f32 and f64 immediates (authored by sunfish).
[WebAssembly] Avoid `bit_cast` when printing f32 and f64 immediates
Feb 26 2021, 2:20 PM
sunfish closed D97490: [WebAssembly] Avoid `bit_cast` when printing f32 and f64 immediates.
Feb 26 2021, 2:19 PM · Restricted Project

Feb 25 2021

sunfish requested review of D97490: [WebAssembly] Avoid `bit_cast` when printing f32 and f64 immediates.
Feb 25 2021, 11:17 AM · Restricted Project

Feb 11 2021

sunfish committed rGf9c05fc39145: [WebAssembly] Use the new crt1-command.o if present. (authored by sunfish).
[WebAssembly] Use the new crt1-command.o if present.
Feb 11 2021, 2:45 PM
sunfish closed D89274: [WebAssembly] Use the new crt1-command.o if present..
Feb 11 2021, 2:44 PM · Restricted Project

Feb 10 2021

sunfish added a comment to D89274: [WebAssembly] Use the new crt1-command.o if present..

I don't see a way to do this with weak symbols, and an install script would be yet-another moving part that we'd have to make on end-user systems.

Feb 10 2021, 5:02 PM · Restricted Project
sunfish added a comment to D89274: [WebAssembly] Use the new crt1-command.o if present..

It's to ensure that older LLVM works with newer WASI libc, and newer clang works with older WASI libc. New-style commands require [lld support]. We can assume that if clang is updated, lld has the requisite support.

Feb 10 2021, 3:42 PM · Restricted Project
sunfish added a comment to D89274: [WebAssembly] Use the new crt1-command.o if present..

Ping!

Feb 10 2021, 10:24 AM · Restricted Project

Feb 4 2021

sunfish committed rG95da64da23ac: [WebAssembly] Use single-threaded mode when -matomics isn't enabled. (authored by sunfish).
[WebAssembly] Use single-threaded mode when -matomics isn't enabled.
Feb 4 2021, 6:17 PM
sunfish closed D96091: [WebAssembly] Use single-threaded mode when -matomics isn't enabled..
Feb 4 2021, 6:17 PM · Restricted Project
sunfish committed rG698c6b0a099b: [WebAssembly] Support single-floating-point immediate value (authored by sunfish).
[WebAssembly] Support single-floating-point immediate value
Feb 4 2021, 6:06 PM
sunfish closed D77384: [WebAssembly] Support single-floating-point immediate value.
Feb 4 2021, 6:06 PM · Restricted Project
sunfish requested review of D96091: [WebAssembly] Use single-threaded mode when -matomics isn't enabled..
Feb 4 2021, 4:16 PM · Restricted Project

Jan 4 2021

sunfish accepted D77384: [WebAssembly] Support single-floating-point immediate value.

Looks good!

Jan 4 2021, 7:04 AM · Restricted Project

Oct 13 2020

sunfish abandoned D89293: [WebAssembly] Don't GC constructors from libraries under --whole-archive.

Abandoned in favor of https://reviews.llvm.org/D89290 .

Oct 13 2020, 5:53 AM · Restricted Project

Oct 12 2020

sunfish added a comment to D89293: [WebAssembly] Don't GC constructors from libraries under --whole-archive.

Either patch works for me. Yours is shorter :-).

Oct 12 2020, 9:17 PM · Restricted Project
sunfish accepted D89290: [lld][WebAssembly] Don't GC library objects under `--whole-archive`.

LGTM; thanks for fixing this!

Oct 12 2020, 9:16 PM · Restricted Project
sunfish added a comment to D85062: [WebAssembly] GC constructor functions in otherwise unused archive objects.

Fix (I believe) is in https://reviews.llvm.org/D89290

Oct 12 2020, 9:15 PM · Restricted Project
sunfish requested review of D89293: [WebAssembly] Don't GC constructors from libraries under --whole-archive.
Oct 12 2020, 9:13 PM · Restricted Project
sunfish committed rG950ae4309112: [WebAssembly] GC constructor functions in otherwise unused archive objects (authored by sunfish).
[WebAssembly] GC constructor functions in otherwise unused archive objects
Oct 12 2020, 6:55 PM
sunfish closed D85062: [WebAssembly] GC constructor functions in otherwise unused archive objects.
Oct 12 2020, 6:55 PM · Restricted Project
sunfish requested review of D89274: [WebAssembly] Use the new crt1-command.o if present..
Oct 12 2020, 2:57 PM · Restricted Project
sunfish added a comment to D85062: [WebAssembly] GC constructor functions in otherwise unused archive objects.

I've finished addressing the review comments, so this is now ready for review again.

Oct 12 2020, 2:53 PM · Restricted Project

Sep 30 2020

sunfish added inline comments to D85062: [WebAssembly] GC constructor functions in otherwise unused archive objects.
Sep 30 2020, 8:39 PM · Restricted Project
sunfish updated the diff for D85062: [WebAssembly] GC constructor functions in otherwise unused archive objects.

Address review feedback.

Sep 30 2020, 8:38 PM · Restricted Project
sunfish closed D81689: [WebAssembly] New-style command support.

Thanks! Description updated, fixes applied, and landed in https://reviews.llvm.org/rG6cd8511e5932

Sep 30 2020, 7:06 PM · Restricted Project, Restricted Project
sunfish committed rG6cd8511e5932: [WebAssembly] New-style command support (authored by sunfish).
[WebAssembly] New-style command support
Sep 30 2020, 7:03 PM
sunfish updated the summary of D81689: [WebAssembly] New-style command support.
Sep 30 2020, 5:19 PM · Restricted Project, Restricted Project
sunfish added a comment to D81689: [WebAssembly] New-style command support.

Friendly ping!

Sep 30 2020, 4:20 PM · Restricted Project, Restricted Project

Aug 21 2020

sunfish committed rG50aae463315d: Update my email address. (authored by sunfish).
Update my email address.
Aug 21 2020, 10:16 AM

Aug 5 2020

sunfish accepted D85347: [WebAssembly] Fix types in wasm_simd128.h and add tests.

Ah, it seems the reason I didn't see this is that I wasn't using -flax-vector-conversions=none. Thanks for fixing this!

Aug 5 2020, 12:54 PM · Restricted Project

Aug 4 2020

sunfish added a reviewer for D81689: [WebAssembly] New-style command support: sbc100.
Aug 4 2020, 4:19 PM · Restricted Project, Restricted Project
sunfish added a comment to D76885: [lld][COFF][ELF][WebAssembly] Replace --[no-]threads /threads[:no] with --threads={1,2,...} /threads:{1,2,...}.

I've now opened https://github.com/WebAssembly/wasi-sdk/pull/151/ to remove the --no-threads from wasi-sdk, which was a workaround for an old bug that has long been fixed.

Aug 4 2020, 1:05 PM · Restricted Project

Aug 3 2020

sunfish updated the diff for D85074: [WebAssembly] Use "signed char" instead of "char" in SIMD intrinsics..
  • Update clang/test/CodeGen/builtins-wasm.c.
Aug 3 2020, 6:45 AM · Restricted Project

Aug 2 2020

sunfish updated the diff for D85062: [WebAssembly] GC constructor functions in otherwise unused archive objects.
  • Reorganize the code a little so that we don't have to call mark multiple times.
  • Fix a bug where we weren't considering calls from constructors as keeping other constructors live.
  • Add a few more tests.
Aug 2 2020, 7:34 AM · Restricted Project

Aug 1 2020

sunfish requested review of D85074: [WebAssembly] Use "signed char" instead of "char" in SIMD intrinsics..
Aug 1 2020, 7:53 AM · Restricted Project

Jul 31 2020

sunfish added inline comments to D85062: [WebAssembly] GC constructor functions in otherwise unused archive objects.
Jul 31 2020, 10:06 PM · Restricted Project
sunfish added a comment to D85062: [WebAssembly] GC constructor functions in otherwise unused archive objects.

To be clear this change only relates to object files at are part of ar archives and are not part of the link? Perhaps mention that in the PR title/description.

Jul 31 2020, 7:25 PM · Restricted Project
sunfish retitled D85062: [WebAssembly] GC constructor functions in otherwise unused archive objects from [WebAssembly] GC constructor functions in otherwise unused objects to [WebAssembly] GC constructor functions in otherwise unused archive objects.
Jul 31 2020, 7:24 PM · Restricted Project
sunfish requested review of D85062: [WebAssembly] GC constructor functions in otherwise unused archive objects.
Jul 31 2020, 5:23 PM · Restricted Project
sunfish added inline comments to D81689: [WebAssembly] New-style command support.
Jul 31 2020, 1:27 PM · Restricted Project, Restricted Project
sunfish updated the diff for D81689: [WebAssembly] New-style command support.

Address review feedback.

Jul 31 2020, 1:27 PM · Restricted Project, Restricted Project

Jun 25 2020

sunfish added inline comments to D81760: [WebAssembly] Add warnings for -shared and -pie.
Jun 25 2020, 12:28 PM · Restricted Project
sunfish updated the diff for D81760: [WebAssembly] Add warnings for -shared and -pie.
  • Rename -emscripten-pic to -experimental-pic
  • Change the warning text to say "not stable" rather than "not yet implemented"
Jun 25 2020, 12:28 PM · Restricted Project

Jun 16 2020

sunfish added a comment to D81962: [lld][WebAssembly] Allow ctors functions that return values.

This change looks fine to me, though if you wanted to minimize the amount of code being emitted by the linker, another option would be to look into whether LLVM's FixFunctionBitcasts could handle this -- there should be a bitcast in the llvm.global_ctors array.

Jun 16 2020, 4:28 PM · Restricted Project

Jun 12 2020

sunfish closed D81688: [WebAssembly] WebAssembly doesn't support "protected" visibility.

Landed in 66042959590d6db9d2a12803a16476d4e3508f3f.

Jun 12 2020, 7:55 PM · Restricted Project, Restricted Project
sunfish created D81760: [WebAssembly] Add warnings for -shared and -pie.
Jun 12 2020, 1:07 PM · Restricted Project
sunfish added inline comments to D81689: [WebAssembly] New-style command support.
Jun 12 2020, 1:07 PM · Restricted Project, Restricted Project
sunfish updated the diff for D81689: [WebAssembly] New-style command support.
  • Address review feedback
  • Mark callCtors live in the MarkLive pass rather than in the Writer pass.
Jun 12 2020, 12:35 PM · Restricted Project, Restricted Project
sunfish updated the diff for D81689: [WebAssembly] New-style command support.

Run clang-format and fix lint warnings.

Jun 12 2020, 8:39 AM · Restricted Project, Restricted Project
sunfish updated the diff for D81689: [WebAssembly] New-style command support.

Use the return value of handleUndefined instead of doing a separate lookup, which avoids a redundant lookup and handles LazySymbols properly.

Jun 12 2020, 7:31 AM · Restricted Project, Restricted Project

Jun 11 2020

sunfish created D81689: [WebAssembly] New-style command support.
Jun 11 2020, 1:48 PM · Restricted Project, Restricted Project
sunfish created D81688: [WebAssembly] WebAssembly doesn't support "protected" visibility.
Jun 11 2020, 1:47 PM · Restricted Project, Restricted Project

Jun 4 2020

sunfish added inline comments to D62922: [WebAssembly] Implement "Reactor" mode.
Jun 4 2020, 5:09 PM · Restricted Project, Restricted Project
sunfish added inline comments to D62922: [WebAssembly] Implement "Reactor" mode.
Jun 4 2020, 8:11 AM · Restricted Project, Restricted Project

Jun 3 2020

sunfish added a comment to D59520: [WebAssembly] Address review comments on r352930.

I apologize again for the major delay. I've now updated the patch and addressed all of your comments.

Jun 3 2020, 4:35 PM · Restricted Project
sunfish updated the diff for D59520: [WebAssembly] Address review comments on r352930.
  • Add tests for redeclaration behavior
  • Remove disabled tests (previously marked with FIXMEs)
  • Made the mismatch warning more informative.
Jun 3 2020, 4:35 PM · Restricted Project

May 11 2020

sunfish accepted D77384: [WebAssembly] Support single-floating-point immediate value.

Looks good to me.

May 11 2020, 3:07 PM · Restricted Project

Apr 23 2020

sunfish added a comment to D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries.

It's for users who want smaller wasm binaries. It's not currently documented, though yes, it would be nice to document it.

But how would a user even end up with wasm-opt in the same directory of clang binaries?

Apr 23 2020, 11:21 AM · Restricted Project
sunfish added a comment to D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries.

Are there plans to offer a way to disable this behavior (or have it optional in the first place)?
We'd like to run some custom processing between wasm-ld and wasm-opt which can't happen after the latter due to some of its one-way destructive optimizations (i.e. memory-packing or simplify-globals passes).
The only way now is to tell our users to place wasm-opt somewhere where clang can't find it. Or instead of using one clang super-command to manually call -cc1 and wasm-ld separately which is disappointing.

Also, is it even common to place wasm-opt next to the clang executable? Who is this for? Is this documented?

Apr 23 2020, 7:32 AM · Restricted Project

Apr 13 2020

sunfish added a comment to D62922: [WebAssembly] Implement "Reactor" mode.

This addresses the review feedback from earlier. To answer this question:

Apr 13 2020, 5:57 PM · Restricted Project, Restricted Project
sunfish updated the diff for D62922: [WebAssembly] Implement "Reactor" mode.

Rebase, update, add a test, and add basic error reporting.

Apr 13 2020, 5:57 PM · Restricted Project, Restricted Project

Apr 11 2020

sunfish accepted D77084: [lld][WebAssembly] Add test for --export of empty string.
Apr 11 2020, 6:55 AM · Restricted Project
sunfish added a comment to D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu.

Looks good! I don't have anything to add beyond Thomas' review comments.

Apr 11 2020, 6:55 AM · Restricted Project

Apr 7 2020

sunfish accepted D77627: [WebAssembly][MC] Fix leak of std::string members in MCSymbolWasm.
Apr 7 2020, 10:18 AM · Restricted Project

Apr 6 2020

sunfish added a comment to D77384: [WebAssembly] Support single-floating-point immediate value.

Is looks possible that the test failure was due to an unrelated commit 210f40fe9a30212396311d265904b2d73859c53d. If so, it's possible it's fixed as of 74ab5d98d07f0b0226f45ccca8df6a450d52fb7b.

Apr 6 2020, 8:06 AM · Restricted Project

Apr 4 2020

sunfish added a comment to D77384: [WebAssembly] Support single-floating-point immediate value.

On some 32-bit x86 hosts, particularly those that use x87 arithmetic, loading and storing with floating-point types can change the NaN bit pattern. To preserve the bit pattern reliably, MCOperand should represent floating-point immediates by representing the bits in int32_t for 32-bit and int64_t for 64-bit. Codegen and MC don't usually look at the value, but where they do need to interpret it as a floating-point value, they can reinterpret the bits on demand.

Apr 4 2020, 11:09 AM · Restricted Project

Mar 30 2020

sunfish accepted D76959: [WebAssembly] Import wasm_simd128.h from Emscripten.

Cool, LGTM, with optional suggestion for signed char below:

Mar 30 2020, 4:57 PM · Restricted Project

Mar 27 2020

sunfish added a comment to D76959: [WebAssembly] Import wasm_simd128.h from Emscripten.

Very cool, thanks for putting this together!

Mar 27 2020, 5:39 PM · Restricted Project