[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 cfe-c

[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] 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 LLV

[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] 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 LLV

[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. Repository:

[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 we

[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 a

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

[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 dete

[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 n

[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: /llvm/tools/cla

[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 omitti

[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 https

[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 https://reviews.llv

[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 "$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 `lib

[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&revision=352221 and should not enable _Float16 yet. The test/Preprocessor/init.c tests were semi-automatically generated, so i

[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 https://list

[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] 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] 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 "p

[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 arch-

[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 c

[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 co

[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] 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 https://reviews.llvm.org/D41941/new/

[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] 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 t

[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 ___ cfe-commit

[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 https://reviews.llvm.org/D62

[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 LAS

[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 https://reviews.l

[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)) +NewAttr

[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" "{{.*[/\\]}}libclang_rt.bui

[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" "{{.*[/\\]}}libclang_rt.bui

[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, "protecte

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

[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 happ

[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 same

[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`, -funsig

[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: clang/include/clang/Basic/Builtin

[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 +// RUN

[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 calle

[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 mai

[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 http://lists.llvm

[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 st

[PATCH] D125728: [WebAssembly] Update supported features in -mcpu=generic

sunfish created this revision. Herald added subscribers: pmatos, asb, ecnelises, jgravelle-google, sbc100, dschuff. Herald added a project: All. sunfish requested review of this revision. Herald added a subscriber: aheejin. Herald added a project: clang. Enable nontrapping-fptoint, sign-ext, bulk

[PATCH] D125728: [WebAssembly] Update supported features in -mcpu=generic

sunfish updated this revision to Diff 431064. sunfish added a comment. Add a driver test, and add release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125728/new/ https://reviews.llvm.org/D125728 Files: clang/docs/ReleaseNotes.rst clan

[PATCH] D125728: [WebAssembly] Update supported features in -mcpu=generic

sunfish updated this revision to Diff 431067. sunfish added a comment. Add tests for -mcpu=mvp and -mcpu=bleeding-edge. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125728/new/ https://reviews.llvm.org/D125728 Files: clang/docs/ReleaseNotes.rst

[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 su

[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 th

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

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

[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 not

[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 isn't enabled. (authored by sunfish). Repository: rG LLVM Github Monorepo CHA

[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 https://lists.llvm.org/cgi-

[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 support

[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 crt1

[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: https://reviews.llvm.org/D89274

[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 f

[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 t

[PATCH] D125728: [WebAssembly] Update supported features in -mcpu=generic

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1e4e2433bcd1: [WebAssembly] Update supported features in the generic CPU configuration (authored by sunfish). Changed prior to commit: https://rev

[PATCH] D75277: [WebAssembly] Remove restriction on main name mangling

sunfish added inline comments. Comment at: llvm/lib/Target/WebAssembly/WebAssemblyFixFunctionBitcasts.cpp:264 - } -} } Why is this code going away? This isn't Emscripten-specific code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75277: [WebAssembly] Remove restriction on main name mangling

sunfish added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6360 + auto *GA = llvm::GlobalAlias::create("__main_void", F); + GA->setVisibility(F->getVisibility()); +} Can this use `GA->setVisibility(GlobalValue::HiddenVisibility)`

[PATCH] D75277: [WebAssembly] Remove restriction on main name mangling

sunfish added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6360 + auto *GA = llvm::GlobalAlias::create("__main_void", F); + GA->setVisibility(F->getVisibility()); +} sbc100 wrote: > sunfish wrote: > > Can this use `GA->setVisibil

[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 https://reviews

[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. `export

[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`? Comment

[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 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: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:137 + getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple + + "/llvm-lto"); +} ---

[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 good

[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 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: clang/lib/Driver/ToolChains/Web

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

[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"), export_name

[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] 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] 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: https://git

[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. Th

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

[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 pa

[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` environme

[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. > Un

[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 inconsi

[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 https://github.com/WebAssembly/tool-convention

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

[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 cl

[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] D150803: Add a new `wasm_custom` clang attribute for marking functions.

sunfish added a comment. In D150803#4419436 , @sbc100 wrote: > Did you mean to comment on the old PR? This new PR doesn't propose either > "jspi" or "async", but adds that ability to define custom attributes.. since > that was deemed more flexible and

[PATCH] D153293: [clang][WebAssembly] support wasm32-wasi shared libraries

sunfish added a comment. This patch appears to be just the release notes and the test; is the actual driver change missing? Comment at: clang/docs/ReleaseNotes.rst:706 + limited to the Emscripten target OS and now works with other targets such as + wasm32-wasi. ---

  1   2   >