[PATCH] D28381: [WebAssembly] Always inline atomics

sunfish added a comment. (Emscripten does set the thread model.) Whether WebAssembly should provide 64-bit atomics when on hardware where they're not lock-free is an interesting wasm design question, and the code here and in target-independent parts of clang may need to be changed as a result

[PATCH] D28381: [WebAssembly] Always inline atomics

sunfish added a comment. Since we may soon have people prototyping actual atomics, I'd prefer to find a less invasive way to fix this. Would it be difficult to enable atomic.c in Emscripten's compiler-rt build, to define these libcalls (assuming that the problem is just that they're not

[PATCH] D40738: Don't use Wasm function sections for more than one function

sunfish accepted this revision. sunfish added a comment. This revision is now accepted and ready to land. I think that was copied from LinuxTargetInfo before we figured out our current object file strategy. On native platforms, it's an icache optimization, because startup functions are all

[PATCH] D41073: Wasm: add support in libcxx

sunfish added a comment. Either way. I won't be able to get to it until next week, so feel free to land it earlier. Repository: rCXX libc++ https://reviews.llvm.org/D41073 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41073: Wasm: add support in libcxx

sunfish accepted this revision. sunfish added a comment. This revision is now accepted and ready to land. Yes, wasm has its own object format, so this change makes sense. Repository: rCXX libc++ https://reviews.llvm.org/D41073 ___ cfe-commits

[PATCH] D46805: If some platforms do not support an attribute, we should exclude the platform

sunfish added a comment. In https://reviews.llvm.org/D46805#1115681, @rsmith wrote: > In https://reviews.llvm.org/D46805#1113358, @aaron.ballman wrote: > > > @rsmith -- do the object file formats listed look correct to you? > > > They look at least plausible. We should be able to test whether

[PATCH] D48443: [WebAssembly] Add no-prototype attribute to prototype-less C functions

sunfish accepted this revision. sunfish added a comment. This revision is now accepted and ready to land. I haven't thought through all the possibilities related to `!FD->doesThisDeclarationHaveABody()`, but overall this looks good. Repository: rC Clang https://reviews.llvm.org/D48443

[PATCH] D37831: [WebAssembly] Don't pass -ffunction-section/-fdata-sections

sunfish accepted this revision. sunfish added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D37831#871238, @dschuff wrote: > I think the reasoning was really just that code size reduction is even more > important on wasm than other platforms, and

[PATCH] D43540: [WebAssembly] Enable -Werror=strict-prototypes by default

sunfish created this revision. sunfish added a reviewer: sbc100. Herald added subscribers: llvm-commits, aheejin, jfb. WebAssembly, the architecture, is not designed to support unprototyped calling conventions. For example, clang tends to assume that it can use varargs to implement parts of the

[PATCH] D40526: [WebAssembly] Change size_t to `unsigned long`

sunfish added a comment. Is this related to the issue reported in the thread here ? Repository: rC Clang https://reviews.llvm.org/D40526 ___ cfe-commits mailing list

[PATCH] D41941: [WebAssembly] Change int_fast16_t to 32-bit

sunfish added a comment. As @nw points out, clang's lib/Headers/stdint.h doesn't support this yet. Unless anyone feels strongly, I now propose we close this and just leave int_fast16_t/uint_fast16_t as-is. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D43540: [WebAssembly] Enable -Werror=strict-prototypes by default

sunfish added a comment. With https://reviews.llvm.org/D48471 and related work, it's not less urgent to do this. And since there were objections to having target-specific warnings anyway, let's close this. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43540/new/

[PATCH] D58742: [WebAssembly] Remove uses of ThreadModel

sunfish added a comment. Wasm gives users reasons to want -mthread-model single that other architectures don't, even when -matomics is enabled by default. When shared memory is used, wasm requires modules to declare a max memory size, which is a burden on applications that want to use dynamic

[PATCH] D58742: [WebAssembly] Remove uses of ThreadModel

sunfish added a comment. 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

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish added a comment. In D59520#1434854 , @aaron.ballman wrote: > > 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

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish marked an inline comment as done. sunfish added inline comments. Comment at: test/Sema/attr-wasm.c:3 + +void name_a() {} + aaron.ballman wrote: > Was this intended to be used somewhere? Probably can just be removed if not. No, you're right that we don't

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish created this revision. sunfish added a reviewer: aaron.ballman. Herald added subscribers: aheejin, jgravelle-google, sbc100, dschuff. Herald added a project: clang. This patch addresses the review comments on r352930: - Removes redundant diagnostic checking code - Removes errnoneous use

[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

sunfish added a comment. That sounds reasonable to me too. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57874/new/ https://reviews.llvm.org/D57874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

sunfish added a comment. > - `-matomics` means `-mthread-model posix` The others sound reasonable, though this one seems a little surprising -- a user might have -matomics enabled because they're targeting a VM that has atomics, but still not want to use -mthread-model posix because their code

[PATCH] D57155: [WebAssembly] Add a __wasi__ target macro

sunfish created this revision. sunfish added reviewers: sbc100, dschuff, aheejin. Herald added subscribers: llvm-commits, jgravelle-google. sunfish edited the summary of this revision. This adds a `__wasi__` macro for the wasi OS, similar to `__linux__` etc. for other OS's. Repository: rL

[PATCH] D57160: [WebAssembly] Add an import_module function attribute

sunfish created this revision. sunfish added reviewers: sbc100, dschuff, aheejin. Herald added subscribers: llvm-commits, jgravelle-google. This adds a C/C++ attribute which corresponds to the LLVM IR wasm-import-module attribute. It allows code to specify an explicit import module.

[PATCH] D57577: Make predefined FLT16 macros conditional on support for the type

sunfish added a comment. I think WebAssembly is in the same situation as most other architectures, as discussed here: http://llvm.org/viewvc/llvm-project?view=revision=352221 and should not enable _Float16 yet. The test/Preprocessor/init.c tests were semi-automatically generated, so it wasn't

[PATCH] D63030: [WebAssembly] Modernize include path handling

sunfish accepted this revision. sunfish added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63030/new/ https://reviews.llvm.org/D63030

[PATCH] D62406: [WebAssembly] Use "linker" as linker shortname.

sunfish accepted this revision. sunfish added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62406/new/ https://reviews.llvm.org/D62406

[PATCH] D61338: [WebAssembly] Use the "wasm32-wasi" triple in tests

sunfish created this revision. sunfish added reviewers: dschuff, sbc100, aheejin. Herald added a subscriber: jgravelle-google. Herald added a project: clang. Similar to https://reviews.llvm.org/D61334, update clang tests to use the "wasm32-wasi" triple, removing the "-musl" environment and

[PATCH] D61389: Bump DIAG_SIZE_SEMA up to 4000

sunfish created this revision. sunfish added a reviewer: aaron.ballman. Herald added a subscriber: aheejin. Herald added a project: clang. https://reviews.llvm.org/D59520 adds a few new diagnostics, which happens to run into a limit with DIAG_SIZE_SEMA. As suggested here

[PATCH] D61389: Bump DIAG_SIZE_SEMA up to 4000

sunfish abandoned this revision. sunfish added a comment. Cool :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61389/new/ https://reviews.llvm.org/D61389 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish updated this revision to Diff 197215. sunfish added a comment. Implemented proper diagnostics for import_name/import_module on functions with definitions, and updated the test. I'm unsure of the `DIAG_SIZE_SEMA` change, but without it, the build fails with this error:

[PATCH] D61452: [WebAssembly] Always include /lib in library path

sunfish added a comment. 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. I'd like to keep our sysroots tidy when we can. If some libraries are installed in

[PATCH] D61452: [WebAssembly] Always include /lib in library path

sunfish added a comment. 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

[PATCH] D61452: [WebAssembly] Always include /lib in library path

sunfish added a comment. 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D64537: [WebAssembly] Implement thread-local storage for non-PIC cases

sunfish added a comment. This looks nice! > __wasm_init_tls(calloc(__builtin_wasm_tls_size(), 1)); Would it make sense to change the API contract for `__wasm_init_tls` to guarantee that initializes all bytes (with zeros as needed)? `__wasm_init_tls` knows what bytes it's initializing, so we

[PATCH] D66035: [WebAssembly] WIP: Add support for reference types

sunfish added a comment. 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

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

sunfish added a comment. In D66983#1651981 , @craig.topper wrote: > DAG combine is supposed to check with TargetLowering::isShuffleMaskLegal. In @tlively's example, it is DAGCombine, and it does check isShuffleMaskLegal. However for wasm, it appears

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

sunfish added a comment. Can you say what leads wasm users to maintain such an expectation when, for example, ARM users and x86 users

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

sunfish added a comment. In D66983#1651977 , @dschuff wrote: > 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

[PATCH] D67739: [WebAssembly] Let users know that wasm64 does not exist

sunfish added a comment. Would it be better to do this check in LLVM, in the backend, with a `report_fatal_error`? Clang for its part already supports what we expect wasm64 will need already, and we have a bunch of tests for it, so it'd be nice to keep that code around and tested. And, doing

[PATCH] D64900: [WebAssembly] Implement __builtin_wasm_tls_base intrinsic

sunfish added inline comments. Comment at: cfe/trunk/include/clang/Basic/BuiltinsWebAssembly.def:34 TARGET_BUILTIN(__builtin_wasm_tls_size, "z", "nc", "bulk-memory") +TARGET_BUILTIN(__builtin_wasm_tls_base, "v*", "n", "bulk-memory") This can also be "p" for

[PATCH] D61452: [WebAssembly] Always include /lib in library path

sunfish added a comment. > 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

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

sunfish added a comment. > Do you think that setting this attribute should also prevent GC? That sounds right to me. > I could split this up into clang, llvm, and lld parts if it makes reviewing > simpler. For me, it's easier to have it all together in one. In this patch, `export_name` has a

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

sunfish added a comment. 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"),

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

sunfish added a comment. In D70500#1756011 , @dschuff wrote: > 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.

[PATCH] D71493: [WebAssembly] Setting export_name implies no_dead_strip

sunfish added inline comments. Comment at: llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:725 TOut.emitExportName(WasmSym, ExportName); + Out.EmitSymbolAttribute(WasmSym, MCSA_NoDeadStrip); } It feels like this is a little

[PATCH] D70700: [WebAssembly] Mangle the argc/argv `main` as `__wasm_argc_argv`

sunfish updated this revision to Diff 234426. sunfish added reviewers: sbc100, dschuff, aheejin. sunfish added a comment. This updates the `main` vs `__main_argc_argv` patch so that it doesn't apply to Emscripten targets, following the discussion in

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

sunfish abandoned this revision. sunfish added a comment. 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D70780: [WebAssembly] Find wasm-opt with GetProgramPath

sunfish created this revision. sunfish added reviewers: dschuff, ilya-biryukov. Herald added subscribers: cfe-commits, aheejin, jgravelle-google, sbc100. Herald added a project: clang. Instead of just searching for wasm-opt in PATH, use `GetProgramPath`, which checks the `COMPILER_PATH`

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

sunfish added a comment. In D70500#1759831 , @ilya-biryukov wrote: > 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. >

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

sunfish marked an inline comment as done. sunfish added a comment. In D70500#1757735 , @thakis wrote: > Please don't add code to the driver that runs programs off PATH. Nothing else > does this. Clang's normal `GetProgramPath` does do this:

[PATCH] D70677: [WebAssembly] Change the llvm-lto dir to use the LLVM Version

sunfish created this revision. sunfish added reviewers: sbc100, dschuff. Herald added subscribers: cfe-commits, dexonsmith, aheejin, jgravelle-google, inglorion. Herald added a project: clang. Using the version instead of the VCS revision, which isn't available when LLVM_APPEND_VC_REV is set.

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

sunfish added a comment. I've now posted https://reviews.llvm.org/D70677 which should fix the test failure when `LLVM_APPEND_VC_REV=NO` is set. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70500/new/ https://reviews.llvm.org/D70500

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

sunfish created this revision. Herald added subscribers: cfe-commits, dexonsmith, aheejin, jgravelle-google, inglorion, sbc100, mehdi_amini, dberris, dschuff. Herald added a project: clang. This allows LTO-enabled builds of compiler-rt to be used. Repository: rG LLVM Github Monorepo

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

sunfish marked an inline comment as done. sunfish 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";

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

sunfish updated this revision to Diff 230350. sunfish added a comment. Use PATH instead of WASM_OPT to find wasm-opt. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70500/new/ https://reviews.llvm.org/D70500 Files:

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

sunfish marked an inline comment as done. sunfish 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";

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

sunfish updated this revision to Diff 230322. sunfish added a comment. In D70500#1754012 , @sbc100 wrote: > 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

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

sunfish created this revision. sunfish added reviewers: sbc100, dschuff, aheejin. Herald added subscribers: dexonsmith, steven_wu, hiraditya, jgravelle-google, inglorion, aprantl, mehdi_amini. Herald added a project: clang. When the WASM_OPT environment variable is set, run the wasm-opt tool to

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

sunfish marked an inline comment as done. sunfish added inline comments. Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:137 + getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple + + "/llvm-lto"); +}

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

sunfish marked an inline comment as done. sunfish 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";

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

sunfish marked an inline comment as done. sunfish 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";

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

sunfish marked 2 inline comments as done. sunfish added inline comments. Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:105 +OOpt = "0"; + else if (A->getOption().matches(options::OPT_O)) +OOpt = A->getValue(); dschuff wrote: > This

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

sunfish updated this revision to Diff 230364. sunfish added a comment. Don't run wasm-opt with -O0. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70500/new/ https://reviews.llvm.org/D70500 Files: clang/lib/Driver/ToolChains/WebAssembly.cpp

[PATCH] D70700: [WebAssembly] Mangle the argc/argv `main` as `__wasm_argc_argv`

sunfish created this revision. Herald added subscribers: llvm-commits, dexonsmith, aheejin, jgravelle-google, sbc100, mehdi_amini, dschuff. Herald added projects: clang, LLVM. WebAssembly enforces a rule that caller and callee signatures must match. This means that the traditional technique of

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish updated this revision to Diff 235005. sunfish added a comment. Address review feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59520/new/ https://reviews.llvm.org/D59520 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish marked 10 inline comments as done. sunfish added a comment. I apologize for the extraordinary delays here; at long last, I've now addressed your feedback. Comment at: lib/Sema/SemaDeclAttr.cpp:5781-5783 +Sema::mergeImportNameAttr(Decl *D, SourceRange Range, +

[PATCH] D70700: [WebAssembly] Mangle the argc/argv `main` as `__wasm_argc_argv`

sunfish updated this revision to Diff 234942. sunfish added a comment. 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. Repository:

[PATCH] D70700: [WebAssembly] Mangle the argc/argv `main` as `__main_argc_argv`

sunfish added a comment. @sbc100 Friendly ping :-). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70700/new/ https://reviews.llvm.org/D70700 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

sunfish added a comment. Looks good! I don't have anything to add beyond Thomas' review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77908/new/ https://reviews.llvm.org/D77908 ___

[PATCH] D62922: [WebAssembly] Implement "Reactor" mode

sunfish updated this revision to Diff 257171. sunfish added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Rebase, update, add a test, and add basic error reporting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D62922: [WebAssembly] Implement "Reactor" mode

sunfish added a comment. This addresses the review feedback from earlier. To answer this question: > I assume that all he users -Wl args come after.. so its still possible to set > --entry on the command line? Yes, that is what happens. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D76547: [WebAssembly] Add wasm-exported function attribute

sunfish added a comment. Instead of creating a new LLVM-IR-level attribute here, could you have clang translate the attribute to "wasm-export-name", to keep the LLVM-IR level simpler? Also, I myself would be more comfortable with this change if it were restricted to Emscripten for now.

[PATCH] D76959: [WebAssembly] Import wasm_simd128.h from Emscripten

sunfish added a comment. Very cool, thanks for putting this together! Comment at: clang/lib/Headers/wasm_simd128.h:10 + +#pragma once + Do you know why other clang headers, such as `lib/Headers/xmmintrin.h`, don't use `#pragma once`?

[PATCH] D76959: [WebAssembly] Import wasm_simd128.h from Emscripten

sunfish accepted this revision. sunfish added a comment. This revision is now accepted and ready to land. Cool, LGTM, with optional suggestion for signed char below: Comment at: clang/lib/Headers/wasm_simd128.h:30 +typedef long long __i64x2 __attribute__((__vector_size__(16),

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

sunfish added a comment. In D70500#1998994 , @bernhard wrote: > 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

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

sunfish added a comment. In D70500#1999268 , @bernhard wrote: > > 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

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

sunfish created this revision. sunfish added a reviewer: sbc100. Herald added subscribers: cfe-commits, ecnelises, jgravelle-google, dschuff. Herald added a project: clang. sunfish requested review of this revision. Herald added a subscriber: aheejin. If crt1-command.o exists in the sysroot, the

[PATCH] D85074: [WebAssembly] Use "signed char" instead of "char" in SIMD intrinsics.

sunfish updated this revision to Diff 282608. sunfish added a comment. - Update clang/test/CodeGen/builtins-wasm.c. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85074/new/ https://reviews.llvm.org/D85074 Files:

[PATCH] D85074: [WebAssembly] Use "signed char" instead of "char" in SIMD intrinsics.

sunfish created this revision. sunfish added a reviewer: tlively. Herald added subscribers: jgravelle-google, sbc100, dschuff. Herald added a project: clang. sunfish requested review of this revision. Herald added a subscriber: aheejin. This allows people to use `int8_t` instead of `char`,

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

sunfish accepted this revision. sunfish added a comment. This revision is now accepted and ready to land. Ah, it seems the reason I didn't see this is that I wasn't using `-flax-vector-conversions=none`. Thanks for fixing this! Comment at: clang/test/Headers/wasm.cpp:1 +//

[PATCH] D62922: [WebAssembly] Implement "Reactor" mode

sunfish marked an inline comment as done. sunfish added inline comments. Comment at: clang/test/Driver/wasm-toolchain.c:116 +// CHECK-COMMAND: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]" +// CHECK-COMMAND: wasm-ld{{.*}}" "crt1.o" "[[temp]]" "-lc"

[PATCH] D62922: [WebAssembly] Implement "Reactor" mode

sunfish marked an inline comment as done. sunfish added inline comments. Comment at: clang/test/Driver/wasm-toolchain.c:116 +// CHECK-COMMAND: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]" +// CHECK-COMMAND: wasm-ld{{.*}}" "crt1.o" "[[temp]]" "-lc"

[PATCH] D81688: [WebAssembly] WebAssembly doesn't support "protected" visibility

sunfish created this revision. Herald added subscribers: llvm-commits, aheejin, jgravelle-google, sbc100, dschuff. Herald added projects: clang, LLVM. mplement the `hasProtectedVisibility()` hook to indicate that, like Darwin, WebAssembly doesn't support "protected" visibility. On ELF,

[PATCH] D81688: [WebAssembly] WebAssembly doesn't support "protected" visibility

sunfish closed this revision. sunfish added a comment. Landed in 66042959590d6db9d2a12803a16476d4e3508f3f . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81688/new/

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish updated this revision to Diff 268329. sunfish added a comment. - Add tests for redeclaration behavior - Remove disabled tests (previously marked with FIXMEs) - Made the mismatch warning more informative. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish marked 3 inline comments as done. sunfish added a comment. I apologize again for the major delay. I've now updated the patch and addressed all of your comments. Comment at: clang/lib/Sema/SemaDecl.cpp:2594-2597 + else if (const auto *IMA = dyn_cast(Attr)) +

[PATCH] D104808: [clang][emscripten] Reduce alignof long double from 16 to 8 bytes

sunfish added a comment. 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. One of the assumptions behind this is that it would be ok for malloc to be 16-byte aligned anyway, because SIMD use cases benefit

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

sunfish added inline comments. Comment at: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h:39 + // pointers are lowered to global.get / global.set or local.get / local.set, + // as appropriate. + WASM_ADDRESS_SPACE_MANAGED = 1 tlively wrote: >

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

sunfish added inline comments. Comment at: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h:39 + // pointers are lowered to global.get / global.set or local.get / local.set, + // as appropriate. + WASM_ADDRESS_SPACE_MANAGED = 1 Sorry to throw more

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

sunfish added inline comments. Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1279 + if (const GlobalAddressSDNode *GA = dyn_cast(Op)) +return WebAssembly::isManagedAddressSpace(GA->getAddressSpace()); + sbc100 wrote: > sunfish wrote: >

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

sunfish added inline comments. Comment at: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h:39 + // pointers are lowered to global.get / global.set or local.get / local.set, + // as appropriate. + WASM_ADDRESS_SPACE_MANAGED = 1 tlively wrote: >

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

sunfish 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" It appears this change means

[PATCH] D96091: [WebAssembly] Use single-threaded mode when -matomics isn't enabled.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG95da64da23ac: [WebAssembly] Use single-threaded mode when -matomics isnt enabled. (authored by sunfish). Repository: rG LLVM Github Monorepo

[PATCH] D96091: [WebAssembly] Use single-threaded mode when -matomics isn't enabled.

sunfish created this revision. sunfish added a reviewer: tlively. Herald added subscribers: ecnelises, jfb, jgravelle-google, sbc100, dschuff. sunfish requested review of this revision. Herald added subscribers: cfe-commits, aheejin. Herald added a project: clang. When the -matomics feature is

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

sunfish added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89274/new/ https://reviews.llvm.org/D89274 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

sunfish added a comment. 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. That said, I'm open to other ideas here. [lld

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

sunfish added a comment. 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. How about this: once we reach a point where we don't support the old LLVM anymore, libc can make crt1.o be the same as

[PATCH] D100411: [WebAssembly] Use standard intrinsics for f32x4 and f64x2 ops

sunfish accepted this revision. sunfish added a comment. This revision is now accepted and ready to land. This looks good to me! Could you briefly comment here on what the issue with `llvm.roundeven` is? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D100411: [WebAssembly] Use standard intrinsics for f32x4 and f64x2 ops

sunfish added a comment. Great, thanks! And yes, switching to roundeven for both scalar and SIMD ISel sounds right to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100411/new/ https://reviews.llvm.org/D100411

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf9c05fc39145: [WebAssembly] Use the new crt1-command.o if present. (authored by sunfish). Changed prior to commit:

[PATCH] D121327: Lower `@llvm.global_dtors` using `__cxa_atexit` on MachO

sunfish added a comment. In D121327#3381123 , @yln wrote: > @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

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

sunfish added a comment. I believe wasi-sdk doesn't have this issue, because it has modifications to all headers which define NULL to always use the NULL defined in stddef.h, and it doesn't ship musl's stddef.h. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

  1   2   >