[PATCH] D158140: WIP: [clang-repl] Basic WebAssembly support for running inside a JS engine

sbc100 added a comment. ood idea after all. Is there a suitable place where we could mo Comment at: clang/lib/Interpreter/WASM.cpp:79 + int Result = + lld::wasm::link(LinkerArgs, llvm::outs(), llvm::errs(), false, false); + if (!Result) v.g.vassilev wrot

[PATCH] D156205: wasm: link crt1 even in case of -shared

sbc100 accepted this revision. sbc100 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:80 - const char *Crt1 = "crt1.o"; + bool isCommand = true; + const char *Crt1; LLCM coding s

[PATCH] D57896: Variable names rule

sbc100 added a comment. Herald added a project: All. Is there still appetite to land this change?We made the switch over in LLD a while back without any issues that I know of. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57896/new/ https://re

[PATCH] D150803: [WebAssembly] Support `annotate` clang attributes for marking functions.

sbc100 accepted this revision. sbc100 added a comment. lgtm. Can you update the change description now that we decided to not support values? (Ironically I think have a use for values in pipeline now) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

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

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin, hiraditya, jgravelle-google, dschuff. Herald added a project: clang. sbc100 added reviewers: sunfish, kripken. This matches the existing export-name attribute but exports that symbol but its llvm symbol name.

[PATCH] D76548: docs

sbc100 created this revision. Herald added subscribers: cfe-commits, aheejin, dschuff. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D76548 Files: clang/include/clang/Basic/AttrDocs.td Index: clang/include/clang/Basic/AttrDocs.td ==

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin, mgorny, dschuff. Herald added a project: clang. sbc100 retitled this revision from "Allow -DDEFAULT_SYSROOT to be a relative path" to "[clang] Allow -DDEFAULT_SYSROOT to be a relative path". sbc100 added reviewe

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

sbc100 added a comment. I have no idea how to write a test for this, but I tested it locally with wasi SDK. Hopefully fucia will find this useful too as a generic version of https://reviews.llvm.org/D42019 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D76717: feedback

sbc100 created this revision. Herald added subscribers: cfe-commits, aheejin. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D76717 Files: clang/lib/Driver/Driver.cpp Index: clang/lib/Driver/Driver.cpp ===

[PATCH] D76717: feedback

sbc100 updated this revision to Diff 252723. sbc100 added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76717/new/ https://reviews.llvm.org/D76717 Files: clang/lib/Driver/Driver.cpp Index: clang/lib/Driver/Driver.cpp

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

sbc100 added a comment. Hmm.. `arc diff` was behaving strangely for me and failed to update this PR as I expected. Should fixed now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76653/new/ https://reviews.llvm.org/D76653 _

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

sbc100 updated this revision to Diff 252725. sbc100 added a comment. squash Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76653/new/ https://reviews.llvm.org/D76653 Files: clang/CMakeLists.txt clang/lib/Driver/Driver.cpp Index: clang/lib/Dri

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

sbc100 updated this revision to Diff 252724. sbc100 added a comment. feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76653/new/ https://reviews.llvm.org/D76653 Files: clang/lib/Driver/Driver.cpp Index: clang/lib/Driver/Driver.cpp ==

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

sbc100 marked 3 inline comments as done. sbc100 added inline comments. Comment at: clang/lib/Driver/Driver.cpp:143 + if (llvm::sys::path::is_relative(SysRoot)) { +// Prepend InstalledDir if SysRoot is relative phosek wrote: > Does this return `true` or `fa

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

sbc100 updated this revision to Diff 252909. sbc100 added a comment. feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76653/new/ https://reviews.llvm.org/D76653 Files: clang/CMakeLists.txt clang/lib/Driver/Driver.cpp Index: clang/lib/D

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

This revision was automatically updated to reflect the committed changes. sbc100 marked an inline comment as done. Closed by commit rG0731372ee25c: [clang] Allow -DDEFAULT_SYSROOT to be a relative path (authored by sbc100). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

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

sbc100 added a comment. What about your idea of using the `default` keyword rather than adding a new clang attr? I quite liked that approach. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76547/new/ https://reviews.llvm.org/D76547

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

sbc100 added a comment. I was thinking of dropping the new clang attr in favor of the `default` keyword in the current attr, but actually keeping the llvm attr, since it corresponds quite nicely to the existing EXPORTED symbol attribute in the binary format and avoid duplication in the `.ll`, `

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

sbc100 added inline comments. Comment at: clang/lib/Headers/wasm_simd128.h:10 + +#pragma once + tlively wrote: > sunfish wrote: > > Do you know why other clang headers, such as `lib/Headers/xmmintrin.h`, > > don't use `#pragma once`? > No, I don't know. Accordin

[PATCH] D77115: [WebAssembly] Emit .llvmcmd and .llvmbc as custom sections

sbc100 created this revision. Herald added subscribers: cfe-commits, jfb, sunfish, aheejin, hiraditya, jgravelle-google, dschuff. Herald added a project: clang. sbc100 added reviewers: alexcrichton, sunfish. Fixes: https://bugs.llvm.org/show_bug.cgi?id=45362 Repository: rG LLVM Github Monorep

[PATCH] D77115: [WebAssembly] Emit .llvmcmd and .llvmbc as custom sections

sbc100 added a comment. In D77115#1952536 , @alexcrichton wrote: > Seems reasonable to me! In my testing though if these existed as custom > sections they'd still make their way to the final binary through LLD, so > could LLD skip over these sectiosn by

[PATCH] D77115: [WebAssembly] Emit .llvmcmd and .llvmbc as custom sections

sbc100 updated this revision to Diff 253935. sbc100 added a comment. strip in linker Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77115/new/ https://reviews.llvm.org/D77115 Files: clang/test/Driver/embed-bitcode-wasm.c clang/test/Driver/fembe

[PATCH] D77115: [WebAssembly] Emit .llvmcmd and .llvmbc as custom sections

sbc100 updated this revision to Diff 253941. sbc100 added a comment. Avoid resuing IsUsedInReloc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77115/new/ https://reviews.llvm.org/D77115 Files: clang/test/Driver/embed-bitcode-wasm.c clang/test/

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

sbc100 added a comment. Would you mind re-basing? I'd like to download and try this but it seems to have conflicts right now. (i'll do my best to resolve them you might do better). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70700/new/ https:

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

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin, jgravelle-google, dschuff. Herald added a project: clang. Emscripten now handles/supports this new mode. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D75277 Files: clang/lib/AST/Mangle.c

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

sbc100 added a comment. Kinds seems like two different changes here. But lgtm. 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

sbc100 added a comment. On I just remember why this is probably a bad idea. llvm bitcode is not designed to be stable, unlike object files, so its probably not a good idea to encourage the distributing of bitcode files in SDKs and such.Unless you stronly disagree maybe a good idea to spl

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

sbc100 added a comment. In D70500#1754032 , @dschuff wrote: > If the SDK is distributed with the compiler and version-locked, it seems like > it should be ok. Right, but if a given SDK chooses to do that, it can always at its own `-L` flags. I'm not

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

sbc100 created this revision. Herald added subscribers: llvm-commits, cfe-commits, sunfish, aheejin, hiraditya, jgravelle-google, dschuff. Herald added projects: clang, LLVM. sbc100 added a comment. sbc100 added reviewers: dschuff, sunfish. First stab at getting this attribute plumbed all the way

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

sbc100 added a comment. First stab at getting this attribute plumbed all the way through. Do you think that setting this attribute should also prevent GC? I could split this up into clang, llvm, and lld parts if it makes reviewing simpler. Repository: rG LLVM Github Monorepo CHANGES SINCE

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

sbc100 added inline comments. Comment at: clang/test/Driver/wasm-toolchain-lto.c:6 +// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]" +// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/ Include the final path segment in the expe

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

sbc100 updated this revision to Diff 231618. sbc100 marked an inline comment as done. sbc100 added a comment. - Add support for asm parsing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70520/new/ https://reviews.llvm.org/D70520 Files: clang/inc

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

sbc100 added a comment. In D70520#1754500 , @sunfish wrote: > 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: > >

[PATCH] D70877: [WebAssebmly][MC] Support .import_name/.import_field asm directives

sbc100 created this revision. Herald added subscribers: llvm-commits, cfe-commits, aheejin, hiraditya, jgravelle-google, dschuff. Herald added projects: clang, LLVM. Convert the MC test to use asm rather than bitcode. This is a precursor to https://reviews.llvm.org/D70520. Repository: rG LLV

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

sbc100 updated this revision to Diff 231620. sbc100 added a comment. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70520/new/ https://reviews.llvm.org/D70520 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.

[PATCH] D70877: [WebAssebmly][MC] Support .import_name/.import_field asm directives

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

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

sbc100 added a comment. The first option seems reasonable to me. That is, adding a flag that controls whether the PATH is searched for all tools. Hermetic builds would then pass this flag to prevent PATH from playing any role. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D70877: [WebAssebmly][MC] Support .import_name/.import_field asm directives

sbc100 marked an inline comment as done. sbc100 added inline comments. Comment at: llvm/test/MC/WebAssembly/import-module.s:13 + .import_module foo, bar + .import_name foo, qux + dschuff wrote: > What should happen if there's a directive that refers to a symb

[PATCH] D70877: [WebAssebmly][MC] Support .import_name/.import_field asm directives

sbc100 marked an inline comment as done. sbc100 added inline comments. Comment at: llvm/test/MC/WebAssembly/import-module.s:13 + .import_module foo, bar + .import_name foo, qux + sbc100 wrote: > dschuff wrote: > > What should happen if there's a directive tha

[PATCH] D70877: [WebAssebmly][MC] Support .import_name/.import_field asm directives

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

[PATCH] D70877: [WebAssebmly][MC] Support .import_name/.import_field asm directives

This revision was automatically updated to reflect the committed changes. Closed by commit rGb4f4e370b59a: [WebAssebmly][MC] Support .import_name/.import_field asm directives (authored by sbc100). Changed prior to commit: https://reviews.llvm.org/D70877?vs=231619&id=232660#toc Repository: rG

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

sbc100 updated this revision to Diff 232673. sbc100 added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70520/new/ https://reviews.llvm.org/D70520 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td

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

sbc100 added a comment. Ping .. I think this is good to go now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70520/new/ https://reviews.llvm.org/D70520 ___ cfe-commits mailing list cfe-commits@lists.l

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

sbc100 marked an inline comment as done. sbc100 added inline comments. Comment at: lld/test/wasm/export-name.ll:20 +; CHECK-NEXT:Exports: +; CHECK-NEXT: - Name:memory +; CHECK-NEXT:Kind:MEMORY dschuff wrote: > does this te

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

This revision was automatically updated to reflect the committed changes. sbc100 marked an inline comment as done. Closed by commit rG881d877846e2: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export… (authored by sbc100). Changed prior to commit: https://reviews.llv

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

sbc100 created this revision. Herald added subscribers: llvm-commits, cfe-commits, sunfish, aheejin, hiraditya, jgravelle-google, dschuff. Herald added projects: clang, LLVM. sbc100 added reviewers: dschuff, sunfish. This change updates the clang front end to add symbols to llvm.used when they ha

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

sbc100 updated this revision to Diff 234144. sbc100 added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71493/new/ https://reviews.llvm.org/D71493 Files: clang/lib/Sema/SemaDeclAttr.cpp clang/test/CodeGen/wasm-export-name.c

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

sbc100 marked an inline comment as done. sbc100 added inline comments. Comment at: llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:725 TOut.emitExportName(WasmSym, ExportName); + Out.EmitSymbolAttribute(WasmSym, MCSA_NoDeadStrip); } ---

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

sbc100 updated this revision to Diff 234160. sbc100 added a comment. - revert part Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71493/new/ https://reviews.llvm.org/D71493 Files: clang/lib/Sema/SemaDeclAttr.cpp clang/test/CodeGen/wasm-export-n

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG0a1e349a7933: [WebAssembly] Setting export_name implies llvm.used (authored by sbc100). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71493/new/ https://rev

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

sbc100 added a comment. __wasm_argc_argv -> __main_argc_argv in the title? 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-comm

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

sbc100 added a comment. Wasm is not an acronym so we don't usually display it in uppercase. So yes, I think we should change WasmObjectFile::getFileFormatName() to return lowercase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74433/new/ https:/

[PATCH] D73320: [WebAssembly] Add reference types target feature

sbc100 accepted this revision. sbc100 added inline comments. This revision is now accepted and ready to land. Comment at: llvm/test/CodeGen/WebAssembly/reference-types.ll:4 +; This currently only tests if the command line argument for reference types +; feature is accepted by llc

[PATCH] D45102: Fix typo in comment -fmath-errno=0 -> -fno-math-errno

sbc100 created this revision. Herald added subscribers: cfe-commits, aheejin. The former is not a valid clang argument Repository: rC Clang https://reviews.llvm.org/D45102 Files: include/clang/Basic/Builtins.def Index: include/clang/Basic/Builtins.def

[PATCH] D39354: [WebAssembly] Add crt1.o when calling lld

sbc100 updated this revision to Diff 120529. sbc100 added a comment. - rebase https://reviews.llvm.org/D39354 Files: lib/Driver/ToolChain.cpp lib/Driver/ToolChains/WebAssembly.cpp test/Driver/wasm-toolchain.c Index: test/Driver/wasm-toolchain.c ==

[PATCH] D39748: [WebAssembly] Include GENERIC_TF_SOURCES

sbc100 added a comment. I guess if we make that change we can revert this one? https://reviews.llvm.org/D39748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39748: [WebAssembly] Include GENERIC_TF_SOURCES

sbc100 added a comment. My guess was that it was "long-double-is-f128" that was causing clang to generate references to these symbols, but I wasn't totally sure.. its only aarch64 and mips64 that seem to require these thus far. https://reviews.llvm.org/D39748 ___

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

sbc100 updated this revision to Diff 507802. sbc100 added a comment. Herald added a reviewer: aaron.ballman. Herald added subscribers: llvm-commits, pmatos, asb, wingo, ecnelises. Herald added projects: LLVM, All. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

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

sbc100 added a comment. In D76547#1945094 , @sbc100 wrote: > What about your idea of using the `default` keyword rather than adding a new > clang attr? I quite liked that approach. IIRC I tried this approach but wan't able to make it works since a singl

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

sbc100 updated this revision to Diff 507807. sbc100 added a comment. - update test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76547/new/ https://reviews.llvm.org/D76547 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/Attr

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

sbc100 added a comment. I've limited to new attribute to only the emcripten triple. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76547/new/ https://reviews.llvm.org/D76547 ___ cfe-commits mailing list c

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

sbc100 updated this revision to Diff 507826. sbc100 edited the summary of this revision. sbc100 added a comment. - limit to emscripten Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76547/new/ https://reviews.llvm.org/D76547 Files: clang/include/

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

sbc100 added a comment. I just figured out that this cannot replace the current use of `__attribute__((used))` in emscripten because function attributes only work for functions and we need this mechanism to work for global data addresses too. There is simply no way to do something like `Fn-

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

sbc100 added a comment. Herald added a subscriber: ormris. Nice! lgtm with a couple of nits Comment at: clang/docs/ReleaseNotes.rst:705 +- The `wasm32-wasi` target now supports `Emscripten-style shared libraries +

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

sbc100 accepted this revision. sbc100 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/D153293/new/ https://reviews.llvm.org/D153293 __

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

sbc100 added a comment. I'm happy to merge it, but perhaps we should get @sunfish to lg too. Also, should we remove the `-experimental-pic` linker flag, or are you OK with that warning from the linker? If yes, I wonder if we should do that as part of this CL or a followup? Repository: rG

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

sbc100 added a comment. In D153293#4437644 , @dicej wrote: > In D153293#4436629 , @sbc100 wrote: > >> I'm happy to merge it, but perhaps we should get @sunfish to lg too. >> >> Also, should we remove the `-experim

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

sbc100 added inline comments. Comment at: llvm/utils/lit/lit/llvm/config.py:347 +return triple + m = re.match(r"(\w+)-(\w+)-(\w+)", triple) dicej wrote: > sbc100 wrote: > > Are the the changes to this file meant to be part of this CL? > The `

[PATCH] D150803: Add a new `wasm_custom` clang attribute for marking functions.

sbc100 added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:5608-5612 +Clang supports the ``__attribute__((wasm_async))`` +attribute for the WebAssembly target. This attribute may be attached to a +function definition, which indicates the function will be used

[PATCH] D150803: Add a new `wasm_custom` clang attribute for marking functions.

sbc100 added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:5608-5612 +Clang supports the ``__attribute__((wasm_async))`` +attribute for the WebAssembly target. This attribute may be attached to a +function definition, which indicates the function will be used

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG55e199a2c9f4: [clang][WebAssembly] Support wasm32-wasi shared libraries (authored by dicej, committed by sbc100). Changed prior to commit: https:/

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

sbc100 added a comment. OK to land this? (With a provision that I will add something to the emscripten changelog?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151820/new/ https://reviews.llvm.org/D151820 ___

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

sbc100 abandoned this revision. sbc100 added a comment. Yes, on further investigation is seems that there is precedent for having `alignof(max_align_t)` and `__BIGGEST_ALIGNMENT__` be different. See https://github.com/emscripten-core/emscripten/pull/19728 Repository: rG LLVM Github Monorepo

[PATCH] D150803: Add a new `wasm_custom` clang attribute for marking functions.

sbc100 added a comment. In D150803#4454164 , @brendandahl wrote: > In D150803#4440802 , @aaron.ballman > wrote: > >> Marking as requested changes so it's clear there's more worth discussing, so >> we don't acci

[PATCH] D150803: [WebAssembly] Support `annotate` clang attributes for marking functions.

sbc100 added inline comments. Comment at: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:585-598 +// The fifth field is an optional pointer to the arguments. Append each +// argument to the name separated by a '.'. +auto *ArgsVar = dyn_cast(CS->getOperand(4)->

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

sbc100 added a comment. The reason `__attribute__((export_name("foo")))` doesn't work in all use cases is that we have a lot of existing code that uses the `EMSCRIPTEN_KEEPALIVE` macro. We also have run into other folks who want to include this is some kind of `FOO_API`, or `EXPORT_API` typ

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

sbc100 added a comment. In D76547#4231476 , @aaron.ballman wrote: > In D76547#4231422 , @sbc100 wrote: > >> The reason `__attribute__((export_name("foo")))` doesn't work in all use >> cases is that we have a lot o

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

sbc100 abandoned this revision. sbc100 added a comment. In D76547#4231604 , @aaron.ballman wrote: > In D76547#4231501 , @sbc100 wrote: > >> In D76547#4231476 , @aaron.ballma

[PATCH] D148181: [Demangle] make llvm::demangle take a StringRef; NFC

sbc100 added inline comments. Comment at: llvm/lib/Demangle/Demangle.cpp:29 -std::string llvm::demangle(const std::string &MangledName) { +std::string llvm::demangle(const StringRef MangledName) { std::string Result; Should `const` be removed? (i.e. is Stri

[PATCH] D148573: Allow -fsanitize=function on all targets

sbc100 added a comment. This change seems to be causing problems on the emscripten auto-roller: https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8780394114149321217/overview Failures show up in ubsan tests and look like this: error: symbol '_Z4testi' unsupported subtracti

[PATCH] D148573: Allow -fsanitize=function on all targets

sbc100 added a comment. (I guess this also mean we are lacking some upstream testing for ubsan + wasm + mc?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148573/new/ https://reviews.llvm.org/D148573 __

[PATCH] D148573: Allow -fsanitize=function on all targets

sbc100 added a comment. In D148573#4361509 , @MaskRay wrote: > In D148573#4361396 , @sbc100 wrote: > >> This change seems to be causing problems on the emscripten auto-roller: >> https://ci.chromium.org/ui/p/ems

[PATCH] D150803: [WebAssembly] Add a new `wasm_async` clang attribute for marking async functions.

sbc100 added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1984 + TargetSpecificAttr { + let Spellings = [Clang<"wasm_async">]; + let Documentation = [WebAssemblyAsyncDocs]; brendandahl wrote: > sbc100 wrote: > > Should we

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

sbc100 created this revision. Herald added subscribers: pmatos, wingo, sunfish, jgravelle-google, dschuff. Herald added a project: All. sbc100 requested review of this revision. Herald added subscribers: cfe-commits, aheejin. Herald added a project: clang. Follow up to https://reviews.llvm.org/D10

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

sbc100 added a comment. In D151820#4385393 , @dschuff wrote: > I guess this is basically the C version of max_align_t so it should match. > but... this still has the potential to break things. True, but I think it's not as likely the break things as that

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

sbc100 added a comment. In D151820#4385393 , @dschuff wrote: > I guess this is basically the C version of max_align_t so it should match. Yes, it should match. Having `__BIGGEST_ALIGNMENT__` be 16 for emscripten doesn't make any sense right now. > bu

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

sbc100 added a comment. In D151820#4385512 , @dschuff wrote: >> I don't think it will change anything in that code since >> `__BIGGEST_ALIGNMENT__ >= XNN_ALLOCATION_ALIGNMENT` will still hold true >> both before and after this change (XNN_ALLOCATION_ALI

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

sbc100 added a comment. In D151820#4385536 , @dschuff wrote: >> I don't think it will since `__BIGGEST_ALIGNMENT__ >= >> XNN_ALLOCATION_ALIGNMENT` will remain true after this change.. so this >> change should have no effect on that code. > > I meant tha

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

sbc100 added a subscriber: ngzhian. sbc100 added a comment. In D151820#4385568 , @sbc100 wrote: > In D151820#4385536 , @dschuff wrote: > >>> I don't think it will since `__BIGGEST_ALIGNMENT__ >= >>> XNN_ALLOCATIO

[PATCH] D149917: [lld][WebAssembly] Add --preserve-features flag

sbc100 created this revision. Herald added subscribers: pmatos, asb, wingo, ecnelises, sunfish, jgravelle-google, dschuff. Herald added a project: All. sbc100 requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, aheejin. Herald added a project: clang. This flag cause

[PATCH] D149917: [lld][WebAssembly] Add --preserve-features flag

sbc100 added a comment. Still needs a test.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149917/new/ https://reviews.llvm.org/D149917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

[PATCH] D149917: [lld][WebAssembly] Add --preserve-features flag

sbc100 added a comment. In D149917#4322367 , @dschuff wrote: > Do we want to make this any more general? In the future we might want to > preserve other sections, e.g. passing optimization or profiling info from > LLVM to Binaryen. Or maybe JSPI info?

[PATCH] D150803: [WebAssembly] Add a new `wasm_async` clang attribute for marking async functions.

sbc100 added a comment. This change looks really nice. I like the new relocation type, I think we would have had to add that sooner or later anyway. My only major concern is making this attribute available outside of emscripten (i.e. wasm32-unknown-emscripten). It seems like we should maybe c

[PATCH] D148835: [clang] removes trailing whitespace

sbc100 added a comment. In D148835#4288151 , @erichkeane wrote: > In D148835#4288148 , @cjdb wrote: > >> In D148835#4286924 , >> @aaron.ballman wrote: >> >>> In D148835#

[PATCH] D148730: [C11] Allow initialization of an atomic-qualified pointer from a null pointer constant

sbc100 added a comment. We bisected a build failure on the emscripten waterfall to this change: https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b878293284529557/overview https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/878293284529557/+/u/B

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

sbc100 added a comment. In D151820#4400795 , @tlively wrote: > In D151820#4385512 , @dschuff wrote: > >> I seem to recall that @tlively and I spent a bunch of time with XNNpack >> chasing down some kind of subtle

[PATCH] D150803: [WebAssembly] Add a new `wasm_async` clang attribute for marking async functions.

sbc100 added a comment. In D150803#4401061 , @sunfish wrote: > wasm_jspi works for me. The problem with that is that we will also likely want to use this attribute for asyncify (which is the current async approach we have today) as well as JSPI (which

[PATCH] D150803: [WebAssembly] Add a new `wasm_async` clang attribute for marking async functions.

sbc100 added a comment. In D150803#4401552 , @sunfish wrote: > In D150803#4401204 , @sbc100 wrote: > >> In D150803#4401061 , @sunfish >> wrote: >> >>> wasm_jspi works for

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

sbc100 added a comment. Can we land this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151820/new/ https://reviews.llvm.org/D151820 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D151820: [clang][WebAssembly] Fix __BIGGEST_ALIGNMENT__ under emscripten

sbc100 added a comment. In D151820#4415754 , @dschuff wrote: >> As far as I can tell the only way this change could break XNNpack if >> XNN_ALLOCATION_ALIGNMENT = 8 is wrongly set there... as long as that is the >> correct value for XNN_ALLOCATION_ALIGN

<    1   2   3   4   >