Page MenuHomePhabricator

sunfish (Dan Gohman)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 12 2013, 11:44 AM (301 w, 27 m)

Recent Activity

Tue, Aug 13

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

I have a slight preference for landing this change too.

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

Mon, Aug 12

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.

Mon, Aug 12, 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
sunfish added a comment to D61323: [WebAssembly] Support EXPLICIT_NAME symbols in llvm-readobj.

Fixed in r359605.

Apr 30 2019, 1:15 PM · Restricted Project
sunfish created D61336: [WebAssembly] WASI support for libcxx.
Apr 30 2019, 1:15 PM · Restricted Project
sunfish committed rG397ca2f22eeb: [WebAssembly] Fix test after r359602 (authored by sunfish).
[WebAssembly] Fix test after r359602
Apr 30 2019, 1:00 PM
sunfish created D61334: [WebAssembly] Test the "wasm32-wasi" triple.
Apr 30 2019, 12:38 PM · Restricted Project
sunfish committed rG3b5b9d0e72ae: [WebAssembly] Support EXPLICIT_NAME symbols in llvm-readobj (authored by sunfish).
[WebAssembly] Support EXPLICIT_NAME symbols in llvm-readobj
Apr 30 2019, 12:33 PM
sunfish committed rG3a7532e645b9: [WebAssembly] Support f16 libcalls (authored by sunfish).
[WebAssembly] Support f16 libcalls
Apr 30 2019, 12:18 PM
sunfish updated the diff for D61323: [WebAssembly] Support EXPLICIT_NAME symbols in llvm-readobj.
  • Convert the test input to yaml.
  • Add ObjectYAML support for EXPLICIT_NAME symbols too.
Apr 30 2019, 11:50 AM · Restricted Project
sunfish created D61323: [WebAssembly] Support EXPLICIT_NAME symbols in llvm-readobj.
Apr 30 2019, 10:01 AM · Restricted Project

Apr 29 2019

sunfish created D61287: [WebAssembly] Support f16 libcalls.
Apr 29 2019, 5:01 PM · Restricted Project
sunfish updated the diff for D59520: [WebAssembly] Address review comments on r352930.

Implemented proper diagnostics for import_name/import_module on functions with definitions, and updated the test.

Apr 29 2019, 4:16 PM · Restricted Project
sunfish committed rG8d6e80f95980: [WebAssembly] Make an assertion message prettier. NFC. (authored by sunfish).
[WebAssembly] Make an assertion message prettier. NFC.
Apr 29 2019, 3:42 PM
sunfish added inline comments to D59521: [WebAssembly] Define the signature for __stack_chk_fail.
Apr 29 2019, 3:42 PM · Restricted Project
sunfish committed rG8306cb5702b3: [WebAssembly] Define the signature for __stack_chk_fail (authored by sunfish).
[WebAssembly] Define the signature for __stack_chk_fail
Apr 29 2019, 2:12 PM
sunfish added a comment to D59521: [WebAssembly] Define the signature for __stack_chk_fail.

I made the error message include the name of the function, and added a test.

Apr 29 2019, 2:11 PM · Restricted Project

Apr 24 2019

sunfish added inline comments to D60812: Don't error on CMAKE_SYSTEM_NAME=Wasi.
Apr 24 2019, 2:25 PM · Restricted Project

Apr 22 2019

sunfish added a comment to D60966: [WebAssembly] Emit br_table for most switch instructions.

The theory here is that br_table represents the performance characteristics of a jump table, while br_if represents the performance characteristics of a branch. In hardware, a small switch with 3 cases is often more efficient with a couple of conditional branches than a jump table.

Apr 22 2019, 8:15 AM · Restricted Project

Apr 16 2019

sunfish added inline comments to D60812: Don't error on CMAKE_SYSTEM_NAME=Wasi.
Apr 16 2019, 10:37 PM · Restricted Project

Mar 19 2019

sunfish added inline comments to D59520: [WebAssembly] Address review comments on r352930.
Mar 19 2019, 5:19 PM · Restricted Project
sunfish added a comment to D59520: [WebAssembly] Address review comments on r352930.

Removes errnoneous use of diag::err_alias_is_definition, which turned out to be ineffective anyway since functions can be defined later in the translation unit and avoid detection.

If you need to keep the prohibition that these attributes not be applied to functions with definitions, there are ways to accomplish that (we merge declarations and can decide what to do at that point if we see a declaration that is a definition). Given that, do you still want to lift this restriction?

Mar 19 2019, 5:19 PM · Restricted Project

Mar 18 2019

sunfish created D59521: [WebAssembly] Define the signature for __stack_chk_fail.
Mar 18 2019, 4:42 PM · Restricted Project
sunfish created D59520: [WebAssembly] Address review comments on r352930.
Mar 18 2019, 4:10 PM · Restricted Project

Mar 14 2019

sunfish added a comment to D59281: [WebAssembly] "atomics" feature requires shared memory.

In the future, atomics will likely be enabled by default, however it will continue to be desirable to be able to select non-shared memory, since non-shared memory doesn't require a maximum size, and since it can make interoperating with JS and other external code simpler. Is it possible to structure this patch in a way that supports this?

When building for non-shared memory, shouldn't input code check for "ifdef _REENTRANT" and not generate any atomic instructions?

Mar 14 2019, 1:34 PM · Restricted Project
sunfish added a comment to D59281: [WebAssembly] "atomics" feature requires shared memory.

In the future, atomics will likely be enabled by default, however it will continue to be desirable to be able to select non-shared memory, since non-shared memory doesn't require a maximum size, and since it can make interoperating with JS and other external code simpler. Is it possible to structure this patch in a way that supports this?

Mar 14 2019, 6:47 AM · Restricted Project

Feb 28 2019

sunfish added a comment to D50277: [WebAssembly] Support for atomic fences.

I've now made an updated post on https://github.com/WebAssembly/tool-conventions/issues/59.

Feb 28 2019, 2:04 PM · Restricted Project
sunfish added a comment to D58742: [WebAssembly] Remove uses of ThreadModel.

Wasm gives users reasons to want -mthread-model single that other architectures don't, even when -matomics is enabled by default.

Feb 28 2019, 10:54 AM · Restricted Project, Restricted Project

Feb 27 2019

sunfish added a comment to D58742: [WebAssembly] Remove uses of ThreadModel.

This is still a little confusing to me. -matomic is supposed to be a subtarget flag, stating that the wasm implementation we will run on supports atomic instructions. -mthread-model posix is about the C++ interpretation -- what style implementation of memory model do we want? In the future, -matomic may become enabled by default, when enough wasm engines generally support it. However, -mthread-model single/posix may still be useful to control independently, because even with wasm engines supporting atomic, there are reasons users might still want to compile their apps single-threaded: access to linear memory with no declared max, lower overall code size, or other things.

Feb 27 2019, 5:46 PM · Restricted Project, Restricted Project

Feb 25 2019

sunfish committed rGc71132c0be4e: [WebAssembly] Properly align fp128 arguments in outgoing varargs arguments (authored by sunfish).
[WebAssembly] Properly align fp128 arguments in outgoing varargs arguments
Feb 25 2019, 9:20 PM
sunfish added inline comments to D58656: [WebAssembly] Properly align fp128 arguments in outgoing varargs arguments.
Feb 25 2019, 8:47 PM · Restricted Project
sunfish created D58656: [WebAssembly] Properly align fp128 arguments in outgoing varargs arguments.
Feb 25 2019, 4:49 PM · Restricted Project

Feb 7 2019

sunfish added a comment to D57874: [WebAssembly] Make thread-related options consistent.
  • -matomics means -mthread-model posix
Feb 7 2019, 5:05 PM · Restricted Project, Restricted Project
sunfish committed rGe086afa7a313: [WebAssembly] Update test output after rL353474. NFC. (authored by sunfish).
[WebAssembly] Update test output after rL353474. NFC.
Feb 7 2019, 2:35 PM
sunfish committed rG29874cea31ce: [WebAssembly] Fix imported function symbol names that differ from their import… (authored by sunfish).
[WebAssembly] Fix imported function symbol names that differ from their import…
Feb 7 2019, 2:04 PM
sunfish committed rG9b84eeaa3ee8: [WebAssembly] Fix imported function symbol names that differ from their import… (authored by sunfish).
[WebAssembly] Fix imported function symbol names that differ from their import…
Feb 7 2019, 2:01 PM
sunfish added a comment to D57874: [WebAssembly] Make thread-related options consistent.

That sounds reasonable to me too.

Feb 7 2019, 11:34 AM · Restricted Project, Restricted Project
sunfish added a comment to D57632: [WebAssembly] Fix imported function symbol names that differ from their import names in the .o format.

Looking good.

Can we can a test in the MC/WebAssembly and lld/test/wasm?

Feb 7 2019, 11:31 AM · Restricted Project
sunfish updated the diff for D57632: [WebAssembly] Fix imported function symbol names that differ from their import names in the .o format.
  • Add MC/WebAssembly and lld tests
  • Set the WASM_SYMBOL_NAME_EXPLICIT flag when a symbols name differs from its import name.
Feb 7 2019, 11:31 AM · Restricted Project

Feb 5 2019

sunfish added inline comments to D57620: Optimize printf calls that don't use 128-bit floating-point values.
Feb 5 2019, 5:58 AM · Restricted Project

Feb 4 2019

sunfish updated the diff for D57620: Optimize printf calls that don't use 128-bit floating-point values.
  • Rename printf_no_Lf to __small_printf.
  • Add more tests.
  • Split out enabling the iprintf optimization for WASI, to be submitted in a separate patch later.
Feb 4 2019, 5:34 PM · Restricted Project
sunfish planned changes to D57620: Optimize printf calls that don't use 128-bit floating-point values.
Feb 4 2019, 5:34 PM · Restricted Project
sunfish updated the diff for D57632: [WebAssembly] Fix imported function symbol names that differ from their import names in the .o format.
  • Use a symbol flag to indicate when a custom symbol name is present.
  • Revert the version number bump.
  • Allow globals and events to have custom symbol names too, for consistency.
Feb 4 2019, 3:13 PM · Restricted Project

Feb 1 2019

sunfish created D57632: [WebAssembly] Fix imported function symbol names that differ from their import names in the .o format.
Feb 1 2019, 5:12 PM · Restricted Project
sunfish created D57620: Optimize printf calls that don't use 128-bit floating-point values.
Feb 1 2019, 2:24 PM · Restricted Project
sunfish added a comment to D57602: [WebAssembly] Add an import_field function attribute.

Do you could also achieve this I guess by renaming pwsix.read function itself to pwsix.__read? But I guess it makes sense to allow the use to control all 3 parts of the name separately.

Feb 1 2019, 12:02 PM · Restricted Project, Restricted Project
sunfish created D57603: [WebAssembly] Add codegen support for the import_field attribute.
Feb 1 2019, 10:38 AM · Restricted Project
sunfish created D57602: [WebAssembly] Add an import_field function attribute.
Feb 1 2019, 10:32 AM · Restricted Project, Restricted Project
sunfish added a comment to D57577: Make predefined FLT16 macros conditional on support for the type.

I think WebAssembly is in the same situation as most other architectures, as discussed here:

Feb 1 2019, 7:00 AM · Restricted Project, Restricted Project

Jan 28 2019

sunfish added a comment to D57323: [WebAssembly] Enable main-function signature rewriting for WASI.
Jan 28 2019, 4:45 PM
sunfish updated the diff for D57323: [WebAssembly] Enable main-function signature rewriting for WASI.
  • Remove the target constraint, allowing the main rewrite logic to work on all wasm32 targets.
  • Remove the now-unneeded "wasm-temporary-workarounds" flag altogether.
  • This uncovered some bugs with wrappers for main when main is only a declaration; fix those as well.
Jan 28 2019, 4:40 PM
sunfish added a comment to D57323: [WebAssembly] Enable main-function signature rewriting for WASI.

I mean with your fix we don't need this to be wasi specific do we?

Jan 28 2019, 3:37 PM
sunfish added a comment to D57323: [WebAssembly] Enable main-function signature rewriting for WASI.

I tried to enable this by default recently but had to revert: https://reviews.llvm.org/D54117.

I'm fine for this to land in the mean time but you might want to be aware of this issue.

Jan 28 2019, 2:06 PM
sunfish created D57323: [WebAssembly] Enable main-function signature rewriting for WASI.
Jan 28 2019, 5:19 AM