Page MenuHomePhabricator

sunfish (Dan Gohman)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 12 2013, 11:44 AM (322 w, 3 d)

Recent Activity

Wed, Jan 15

sunfish added a comment to D70700: [WebAssembly] Mangle the argc/argv `main` as `__main_argc_argv`.

@sbc100 Friendly ping :-).

Wed, Jan 15, 2:52 PM · Restricted Project, Restricted Project

Mon, Jan 13

sunfish retitled D70700: [WebAssembly] Mangle the argc/argv `main` as `__main_argc_argv` from [WebAssembly] Mangle the argc/argv `main` as `__wasm_argc_argv` to [WebAssembly] Mangle the argc/argv `main` as `__main_argc_argv`.
Mon, Jan 13, 9:57 AM · Restricted Project, Restricted Project

Sat, Dec 21

sunfish updated the diff for D59520: [WebAssembly] Address review comments on r352930.

Address review feedback.

Sat, Dec 21, 12:06 AM · Restricted Project
sunfish added a comment to D59520: [WebAssembly] Address review comments on r352930.

I apologize for the extraordinary delays here; at long last, I've now addressed your feedback.

Sat, Dec 21, 12:06 AM · Restricted Project

Fri, Dec 20

sunfish created D71793: [WebAssembly] Support wasm exports with zero-length names..
Fri, Dec 20, 9:49 PM · Restricted Project
sunfish updated the diff for D70700: [WebAssembly] Mangle the argc/argv `main` as `__main_argc_argv`.

To support a transition to the new system, temporarily define a __main_void alias for no-argument main. This allows libc to detect whether it's calling old-style or new-style main and do the right thing.

Fri, Dec 20, 12:11 PM · Restricted Project, Restricted Project

Dec 17 2019

sunfish updated the diff for D70700: [WebAssembly] Mangle the argc/argv `main` as `__main_argc_argv`.

This updates the main vs __main_argc_argv patch so that it doesn't apply to Emscripten targets, following the discussion in https://github.com/WebAssembly/tool-conventions/pull/134.

Dec 17 2019, 5:29 PM · Restricted Project, Restricted Project
sunfish added reviewers for D70685: [WebAssembly] Fix the order of destructors in the LowerGlobalDtors pass.: sbc100, dschuff, aheejin.
Dec 17 2019, 5:29 PM · Restricted Project

Dec 16 2019

sunfish added inline comments to D71493: [WebAssembly] Setting export_name implies no_dead_strip.
Dec 16 2019, 2:20 PM · Restricted Project, Restricted Project

Nov 27 2019

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

I'm not an expert in driver code and how it should behave, but being consistent with how other tools are found definitely looks much better than special-casing a single tool.
Unless there are reasons why wasm-opt should be special, why not use the mechanism used by all other tools?

Nov 27 2019, 6:18 AM · Restricted Project
sunfish created D70780: [WebAssembly] Find wasm-opt with GetProgramPath.
Nov 27 2019, 6:09 AM · Restricted Project

Nov 26 2019

sunfish abandoned D70687: [WebAssembly] Add an llvm-lto path for compiler-rt..

I've done some more experimenting with this, and it turns out not to work very well, because calls to these functions get generated after LTO runs, so I'll abandon this.

Nov 26 2019, 11:27 AM · Restricted Project

Nov 25 2019

sunfish created D70700: [WebAssembly] Mangle the argc/argv `main` as `__main_argc_argv`.
Nov 25 2019, 3:42 PM · Restricted Project, Restricted Project
sunfish created D70687: [WebAssembly] Add an llvm-lto path for compiler-rt..
Nov 25 2019, 11:04 AM · Restricted Project
sunfish created D70685: [WebAssembly] Fix the order of destructors in the LowerGlobalDtors pass..
Nov 25 2019, 11:04 AM · Restricted Project
sunfish created D70677: [WebAssembly] Change the llvm-lto dir to use the LLVM Version.
Nov 25 2019, 9:38 AM · Restricted Project
sunfish added a comment to D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries.

I've now posted https://reviews.llvm.org/D70677 which should fix the test failure when LLVM_APPEND_VC_REV=NO is set.

Nov 25 2019, 9:38 AM · Restricted Project
sunfish added a comment to D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries.

Please don't add code to the driver that runs programs off PATH. Nothing else does this.

Nov 25 2019, 9:19 AM · Restricted Project

Nov 21 2019

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

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.

Nov 21 2019, 4:47 PM · Restricted Project

Nov 20 2019

sunfish added a comment to D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names.

Do you think that setting this attribute should also prevent GC?

Nov 20 2019, 8:41 PM · Restricted Project, Restricted Project
sunfish added a comment to D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names.

It appears this doesn't handle exporting an imported function yet, which is fine for now, but it would be good to issue a warning, because wasm itself is capable of representing this:

void aaa(void) __attribute__((import_module("imp"), import_name("foo"), export_name("bar")));
Nov 20 2019, 8:32 PM · Restricted Project, Restricted Project
sunfish updated the diff for D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries.

Don't run wasm-opt with -O0.

Nov 20 2019, 6:43 PM · Restricted Project
sunfish added inline comments to D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries.
Nov 20 2019, 6:43 PM · Restricted Project
sunfish added inline comments to D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries.
Nov 20 2019, 4:43 PM · Restricted Project
sunfish updated the diff for D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries.

Use PATH instead of WASM_OPT to find wasm-opt.

Nov 20 2019, 4:43 PM · Restricted Project
sunfish added a comment to D70515: [WebAssembly] Create a __stack_limit variable.

Does this need to be wasm global? Or can it just be regular data symbol like globalBase? i.e. do we want fast access to it without a load?

Nov 20 2019, 4:34 PM · Restricted Project
sunfish added inline comments to D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries.
Nov 20 2019, 3:20 PM · Restricted Project
sunfish created D70515: [WebAssembly] Create a __stack_limit variable.
Nov 20 2019, 3:11 PM · Restricted Project
sunfish added inline comments to D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries.
Nov 20 2019, 1:28 PM · Restricted Project
sunfish updated the diff for D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries.

On I just remember why this is probably a bad idea. llvm bitcode is not designed to be stable, unlike object files, so its probably not a good idea to encourage the distributing of bitcode files in SDKs and such.

Nov 20 2019, 1:19 PM · Restricted Project
sunfish added inline comments to D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries.
Nov 20 2019, 12:42 PM · Restricted Project
sunfish added inline comments to D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries.
Nov 20 2019, 11:16 AM · Restricted Project
sunfish created D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries.
Nov 20 2019, 10:39 AM · Restricted Project

Sep 21 2019

sunfish added a comment to D67783: [WebAssembly] Remove unused memory instructions and patterns.

This kind of thing ought to have helped code like

extern int A[];
Sep 21 2019, 5:27 AM · Restricted Project

Sep 18 2019

sunfish added a comment to D67739: [WebAssembly] Let users know that wasm64 does not exist.

Would it be better to do this check in LLVM, in the backend, with a report_fatal_error?

Sep 18 2019, 4:22 PM · Restricted Project

Aug 30 2019

sunfish added a comment to D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic.

DAG combine is supposed to check with TargetLowering::isShuffleMaskLegal.

Aug 30 2019, 4:51 PM · Restricted Project, Restricted Project

Aug 29 2019

sunfish committed rG8cfeeaf9de0b: [CodeGen] Fix lowering for returning the result of an extractvalue (authored by sunfish).
[CodeGen] Fix lowering for returning the result of an extractvalue
Aug 29 2019, 9:34 PM
sunfish added a comment to D66978: [CodeGen] Fix lowering for returning the result of an extractvalue.

Generally LGTM, although the tests might be more readable if you pass -wasm-disable-explicit-locals and -wasm-keep-registers as well. It's also concerning that no one noticed this issue before. Do you know why other targets may not have run into it?

Aug 29 2019, 9:27 PM · Restricted Project
sunfish updated the diff for D66978: [CodeGen] Fix lowering for returning the result of an extractvalue.
  • Use -wasm-disable-explicit-locals -wasm-keep-registers to make the test more readable
Aug 29 2019, 9:27 PM · Restricted Project
sunfish added a comment to D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic.

Oh, interesting I didn't notice that those are implementations of the target-specific intrinsics. I wonder if they do that so they can implement the intrinsics on hardware that doesn't support the corresponding instruction? In a situation other than that I think I'd be surprised if I used a builtin for a particular instruction and got something else.

Aug 29 2019, 6:15 PM · Restricted Project, Restricted Project
sunfish added a comment to D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic.

Can you say what leads wasm users to maintain such an expectation when, for example, ARM users and x86 users don't?

Aug 29 2019, 5:41 PM · Restricted Project, Restricted Project
sunfish committed rG7cb9c8a506f3: [WebAssembly] Implement NO_STRIP (authored by sunfish).
[WebAssembly] Implement NO_STRIP
Aug 29 2019, 3:42 PM
sunfish committed rGda84b688f916: [WebAssembly] Make __attribute__((used)) not imply export. (authored by sunfish).
[WebAssembly] Make __attribute__((used)) not imply export.
Aug 29 2019, 3:40 PM
sunfish created D66978: [CodeGen] Fix lowering for returning the result of an extractvalue.
Aug 29 2019, 3:32 PM · Restricted Project
sunfish added a comment to D66968: [WebAssembly] Implement NO_STRIP.

I'm curious why a user would want to use WASM_SYMBOL_NO_STRIP if it doesn't export the symbol? Maybe for post-linker to do stuff perhaps?

Aug 29 2019, 1:06 PM · Restricted Project
sunfish added inline comments to D66968: [WebAssembly] Implement NO_STRIP.
Aug 29 2019, 1:06 PM · Restricted Project
sunfish updated the diff for D66968: [WebAssembly] Implement NO_STRIP.

Include the basic test case.

Aug 29 2019, 1:00 PM · Restricted Project
sunfish abandoned D62443: [WebAssembly] Move Emscripten-specific behavior under a --emscripten flag.

Abandoning in favor of https://reviews.llvm.org/D66968.

Aug 29 2019, 11:48 AM · Restricted Project
sunfish added a comment to D62542: [WebAssembly] Make Emscripten-specific behavior specific to the Emscripten target.

https://reviews.llvm.org/D66968 is now a patch implementing the lld side of this.

Aug 29 2019, 11:48 AM · Restricted Project
sunfish created D66968: [WebAssembly] Implement NO_STRIP.
Aug 29 2019, 11:42 AM · Restricted Project
Herald added a project to D43675: [WebAssembly] Rename imported/exported memory symbol to __linear_memory: Restricted Project.

"memory" is now a de-facto standard. We may still be able to change it, but we'll now need a transition plan.

Aug 29 2019, 9:57 AM · Restricted Project

Aug 13 2019

sunfish accepted D64961: [libcxxabi] Define _LIBCXXABI_GUARD_ABI_ARM on WebAssembly.

I have a slight preference for landing this change too.

Aug 13 2019, 12:23 PM · Restricted Project, Restricted Project

Aug 12 2019

sunfish added a comment to D66035: [WebAssembly] WIP: Add support for reference types.

x86 uses address spaces starting at 256 and counting up for its architecture-specific address spaces. The docs say "Address spaces 1-255 are currently reserved for user-defined code." so we should consider using 256 here.

Aug 12 2019, 3:59 PM · Restricted Project, Restricted Project

Jul 19 2019

sunfish added a comment to D61452: [WebAssembly] Always include <sysroot>/lib in library path.

This allows for us to fall back from arch-specific to generic headers as needed. The same can be true of libraries. Not all libraries contains compiled code. .so files can also be linker scripts that reference other libraries in which case they can be arch-neutral.

Jul 19 2019, 8:37 AM · Restricted Project
sunfish added a comment to D64961: [libcxxabi] Define _LIBCXXABI_GUARD_ABI_ARM on WebAssembly.

My opinion isn't too strong here either. wasm32 doesn't need 8 bytes for this, and there's an existing ABI knob to use less, so it seems to make sense to use it.

Jul 19 2019, 8:28 AM · Restricted Project, Restricted Project

Jul 18 2019

sunfish added inline comments to D64900: [WebAssembly] Implement __builtin_wasm_tls_base intrinsic.
Jul 18 2019, 12:21 PM · Restricted Project, Restricted Project

Jul 10 2019

sunfish added a comment to D64537: [WebAssembly] Implement thread-local storage (local-exec model).

This looks nice!

Jul 10 2019, 4:10 PM · Restricted Project, Restricted Project

Jul 8 2019

sunfish added a comment to D64322: [WebAssembly] Print error message for llvm.clear_cache intrinsic.

The actual compiler crash here came from the fact that we don't have an entry for llvm.clear_cache in the builtin function signature table. Suppose we added a signature for it. Then, we'd get LLVM's and compiler-rt's default behavior, which would be to call __clear_cache, and ultimately just abort at runtime.

Jul 8 2019, 9:15 PM · Restricted Project
sunfish added a comment to D64322: [WebAssembly] Print error message for llvm.clear_cache intrinsic.

Any code that thinks it needs to "clear the cache" is very likely doing something which won't actually work on wasm. Would it be reasonable to issue a report_fatal_error here?

Jul 8 2019, 7:03 AM · Restricted Project

Jun 16 2019

sunfish added inline comments to rL357195: [WebAssembly] Reland of rL356953 (4dcf3acce6).
Jun 16 2019, 3:25 PM

Jun 8 2019

sunfish accepted D63030: [WebAssembly] Modernize include path handling.

LGTM!

Jun 8 2019, 10:25 PM · Restricted Project, Restricted Project

Jun 5 2019

sunfish committed rGba86f2a22e7a: [WebAssembly] Use Emscripten triples in PIC tests. (authored by sunfish).
[WebAssembly] Use Emscripten triples in PIC tests.
Jun 5 2019, 2:01 PM
sunfish updated the diff for D62542: [WebAssembly] Make Emscripten-specific behavior specific to the Emscripten target.

The PIC portion of this patch is now split off as r362638. This patch now just contains the __attribute__((used))/no_dead_strip changes.

Jun 5 2019, 1:06 PM · Restricted Project
sunfish committed rG53572d0470c9: [WebAssembly] Limit PIC support to the Emscripten target (authored by sunfish).
[WebAssembly] Limit PIC support to the Emscripten target
Jun 5 2019, 12:58 PM
sunfish added a comment to D62542: [WebAssembly] Make Emscripten-specific behavior specific to the Emscripten target.

I was hoping you could have the linker default to exporting NO_STRIP symbols? If you don't like that idea then perhaps would have the "export NO_STRIP symbols" behaviour be emscripten-only in the linker and have it driven by the same signal we end up using for -shared/-pie.

Jun 5 2019, 11:19 AM · Restricted Project
sunfish added a comment to D62922: [WebAssembly] Implement "Reactor" mode.

The wasi-libc PR is here.

Jun 5 2019, 11:12 AM · Restricted Project
sunfish created D62922: [WebAssembly] Implement "Reactor" mode.
Jun 5 2019, 11:03 AM · Restricted Project

Jun 3 2019

sunfish updated the diff for D62443: [WebAssembly] Move Emscripten-specific behavior under a --emscripten flag.
  • Rebase
  • Rename IMPLICITLY_USED to NO_STRIP
Jun 3 2019, 5:03 PM · Restricted Project
sunfish updated the diff for D62542: [WebAssembly] Make Emscripten-specific behavior specific to the Emscripten target.
  • Rename IMPLICITLY_USED to NO_STRIP.
  • Remove isExported and setExported accessors.
Jun 3 2019, 4:59 PM · Restricted Project
sunfish added inline comments to D62542: [WebAssembly] Make Emscripten-specific behavior specific to the Emscripten target.
Jun 3 2019, 4:56 PM · Restricted Project

May 31 2019

sunfish added a comment to D62443: [WebAssembly] Move Emscripten-specific behavior under a --emscripten flag.

I have proposed https://reviews.llvm.org/D62744 which covers most of the changes here.

For -pie/-shared I'd rather not add a --emscripten flag. In fact I'd rather not have such a flag at all. Some background that might help here: The PIC support in LLVM conforms to the (admittedly unstable/experimental) spec specified in https://github.com/WebAssembly/tool-conventions/blob/master/DynamicLinking.md. Note that this document includes both the "emscripten" way of doing things and the future "Planned Changes" way of doing things which is implemented in LLVM. emscripten-wasm-finalize, which is part of binaryen, then converts from the LLVM/official ABI to the emscripten ABI for dynamic linking. I will update the docs to be more clear but as I see it non of the PIC support in LLVM is emscripten-specific.

May 31 2019, 3:03 PM · Restricted Project
sunfish accepted D62744: [WebAssembly] Don't export __data_end and __heap_start by default..

Cool, this is nicer than my version :-).

May 31 2019, 2:30 PM · Restricted Project

May 28 2019

sunfish added inline comments to D50277: [WebAssembly] Support for atomic fences.
May 28 2019, 6:21 PM · Restricted Project
sunfish added a comment to D62443: [WebAssembly] Move Emscripten-specific behavior under a --emscripten flag.

Wouldn't it be better to try to avoid adding a --emscripten?

May 28 2019, 5:00 PM · Restricted Project
sunfish added inline comments to D50277: [WebAssembly] Support for atomic fences.
May 28 2019, 4:44 PM · Restricted Project
sunfish added inline comments to D50277: [WebAssembly] Support for atomic fences.
May 28 2019, 2:49 PM · Restricted Project
sunfish resigned from D44024: [WebAssembly] Validate sections: order, and presence of required CODE.
May 28 2019, 11:54 AM · Restricted Project
sunfish updated the diff for D62443: [WebAssembly] Move Emscripten-specific behavior under a --emscripten flag.

This patch now depends on a corresponding LLVM patch: https://reviews.llvm.org/D62542

May 28 2019, 11:54 AM · Restricted Project
sunfish created D62542: [WebAssembly] Make Emscripten-specific behavior specific to the Emscripten target.
May 28 2019, 11:48 AM · Restricted Project

May 27 2019

sunfish added a comment to D62443: [WebAssembly] Move Emscripten-specific behavior under a --emscripten flag.

imo export-dynamic should still work? All it does it export all symbols that are linked in. My compiler uses and depends on that feature (though I can add emscripten as a flag to the linker, I don't think I should).

May 27 2019, 5:59 AM · Restricted Project

May 24 2019

sunfish created D62443: [WebAssembly] Move Emscripten-specific behavior under a --emscripten flag.
May 24 2019, 9:59 PM · Restricted Project
sunfish added a comment to D50277: [WebAssembly] Support for atomic fences.

If I follow the discussion correctly, there are subtle ABI issues either way, so making fence a RMW now doesn't give us fully a future-proof ABI anyway. Consequently, I would prefer to make this patch conditional on the Emscripten triple, as wasm32-wasi and wasm32-unknown-unknown don't require Emscripten compatibility.

May 24 2019, 5:26 PM · Restricted Project
sunfish accepted D62406: [WebAssembly] Use "linker" as linker shortname..

LGTM

May 24 2019, 10:28 AM · Restricted Project, Restricted Project

May 21 2019

sunfish committed rGa49496fb2a16: [WebAssembly] Add the signature for the new llround builtin function (authored by sunfish).
[WebAssembly] Add the signature for the new llround builtin function
May 21 2019, 4:05 PM
sunfish added a comment to D62210: [WebAssembly] Implement __builtin_return_address for emscripten.

A function like this might be implemented in compiler_rt/libgcc or libc, which might in make future WASI system calls or future wasm language features underneath. An interface like __wasm_return_address doesn't commit to a particular implementation. I'm not suggesting enabling it for other targets now, just asking if it makes sense to pick a name which isn't tied to Emscripten.

May 21 2019, 1:41 PM · Restricted Project
sunfish added a comment to D62210: [WebAssembly] Implement __builtin_return_address for emscripten.

Would it make sense to take this opportunity to rename the library function, from emscripten_return_address to something more general, like __wasm_return_address or __wasm_callsite_identifier or something, so that non-Emscripten environments could implement it too if they wished?

May 21 2019, 12:57 PM · Restricted Project
sunfish created D62207: [WebAssembly] Add the signature for the new llround builtin function.
May 21 2019, 9:37 AM · Restricted Project

May 20 2019

sunfish added a comment to D62153: [WebAssembly] Relax signature checking for undefined functions that are not called directly.

I don't know the lld internals well, but the basic approach here sounds right.

May 20 2019, 10:16 AM · Restricted Project

May 2 2019

sunfish added a comment to D61452: [WebAssembly] Always include <sysroot>/lib in library path.

If "$sysroot/lib" ends up coming to mean "wasm32" because people come to depend on that, then wasm64 may end up needing to be different in a gratuitous way, which I'd like to avoid.

May 2 2019, 3:52 PM · Restricted Project
sunfish added a comment to D61452: [WebAssembly] Always include <sysroot>/lib in library path.

The value of supporting single-arch sysroots is unclear to me. It's always possible to have a sysroot with libraries for just one architecture installed, even with multi-arch paths. Is this just about compatibility with build scripts and tools which are hard-coded to "$sysroot/lib"?

May 2 2019, 12:06 PM · Restricted Project
sunfish added a comment to D61452: [WebAssembly] Always include <sysroot>/lib in library path.

If libraries don't correctly install into multi-arch directories, can we fix the libraries? We do this in the wasi-sdk repo to fix up the libc++ and libc++abi libraries, for example.

May 2 2019, 11:23 AM · Restricted Project

May 1 2019

sunfish abandoned D61389: Bump DIAG_SIZE_SEMA up to 4000.

Cool :)

May 1 2019, 3:41 PM · Restricted Project
sunfish created D61389: Bump DIAG_SIZE_SEMA up to 4000.
May 1 2019, 10:28 AM · Restricted Project
sunfish committed rG3efd6e37e4b7: [WebAssembly] WASI support for libcxx (authored by sunfish).
[WebAssembly] WASI support for libcxx
May 1 2019, 9:46 AM

Apr 30 2019

sunfish committed rG0b0d13a704ad: [WebAssembly] Use the "wasm32-wasi" triple in tests (authored by sunfish).
[WebAssembly] Use the "wasm32-wasi" triple in tests
Apr 30 2019, 4:05 PM
sunfish committed rG8e7a05a4567a: [WebAssembly] Test the "wasm32-wasi" triple (authored by sunfish).
[WebAssembly] Test the "wasm32-wasi" triple
Apr 30 2019, 4:04 PM
sunfish added inline comments to D61336: [WebAssembly] WASI support for libcxx.
Apr 30 2019, 3:58 PM · Restricted Project
sunfish updated the diff for D61336: [WebAssembly] WASI support for libcxx.

Drop the getentropy header file change from this patch.

Apr 30 2019, 3:45 PM · Restricted Project
sunfish added inline comments to D61336: [WebAssembly] WASI support for libcxx.
Apr 30 2019, 3:37 PM · Restricted Project
sunfish created D61338: [WebAssembly] Use the "wasm32-wasi" triple in tests.
Apr 30 2019, 1:42 PM · Restricted Project