[PATCH] D40739: Pass through --undefined to Wasm LLD

sbc100 added a comment. I'm a little confused by this. I was assuming you would do "-Wl,-u,foo" or "-Xlinker".I wasn't aware -u was a valid compiler flag itself. It doesn't show up in --help. Repository: rC Clang https://reviews.llvm.org/D40739

[PATCH] D40739: Pass through --undefined to Wasm LLD

sbc100 added a comment. Oh I see. lgtm. Do we need to update any tests? I see we won't have a wasm-ld.c test yet? We should add one? Repository: rC Clang https://reviews.llvm.org/D40739 ___ cfe-commits mailing list

[PATCH] D40739: Pass through --undefined to Wasm LLD

sbc100 added a comment. By the way, thank you for all these wasm patches! Repository: rC Clang https://reviews.llvm.org/D40739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41073: Wasm: add support in libcxx

sbc100 added a comment. Do you want to land this dan? Or I can if you prefer. Repository: rCXX libc++ https://reviews.llvm.org/D41073 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[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] D40739: Pass through --undefined to Wasm LLD

sbc100 added a comment. Sure I'll land this now. Repository: rC Clang https://reviews.llvm.org/D40739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39218: [WebAssembly] Include libclang_rt.builtins in the standard way

sbc100 added inline comments. Comment at: lib/Driver/ToolChain.cpp:318 + else +OSLibName = getOS(); llvm::sys::path::append(Path, "lib", OSLibName); dschuff wrote: > Is this logic intended to replace what was removed from CommonArgs.cpp? > Should there

[PATCH] D39218: [WebAssembly] Include libclang_rt.builtins in the standard way

sbc100 updated this revision to Diff 120111. sbc100 added a comment. - remove debugging - git squash commit for startup_libs. - update test expectations https://reviews.llvm.org/D39218 Files: lib/Driver/ToolChain.cpp lib/Driver/ToolChains/CommonArgs.cpp

[PATCH] D46443: Add missing cstdalign header

sbc100 updated this revision to Diff 145247. sbc100 added a comment. - revert Repository: rCXX libc++ https://reviews.llvm.org/D46443 Files: include/cstdalign test/libcxx/min_max_macros.sh.cpp test/libcxx/utilities/any/size_and_alignment.pass.cpp Index:

[PATCH] D46443: Add missing cstdalign header

sbc100 created this revision. Herald added subscribers: cfe-commits, christof, aheejin. Repository: rCXX libc++ https://reviews.llvm.org/D46443 Files: include/cassert include/cstdalign test/libcxx/min_max_macros.sh.cpp test/libcxx/utilities/any/size_and_alignment.pass.cpp Index:

[PATCH] D46443: Add missing cstdalign header

sbc100 added a comment. ping Repository: rCXX libc++ https://reviews.llvm.org/D46443 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47614: [WebAssembly] Hide new Wasm EH behind its feature flag

sbc100 added inline comments. Comment at: lib/CodeGen/CGException.cpp:322 const EHPersonality = - getCXXPersonality(getTarget().getTriple(), LangOpts); + getCXXPersonality(getTarget().getTriple(), LangOpts, getTarget()); if ( == ) If the triple

[PATCH] D48106: implemented proto to llvm

sbc100 added a comment. I think this broke the BUILD_SHARED_LIBS=1 build: https://reviews.llvm.org/D48503 Repository: rC Clang https://reviews.llvm.org/D48106 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

sbc100 added a comment. Without the check for `doesThisDeclarationHaveABody()` the attribute will also be added to `baz` which is IIRC is not what we want since the C notions of "prototypeless" only applies to forward declarations I think. Repository: rC Clang

[PATCH] D48503: Fix BUILD_SHARED_LIBS=1 build of libclangHandleLLVM

sbc100 created this revision. Herald added subscribers: cfe-commits, aheejin, mgorny. Repository: rC Clang https://reviews.llvm.org/D48503 Files: tools/clang-fuzzer/handle-llvm/CMakeLists.txt Index: tools/clang-fuzzer/handle-llvm/CMakeLists.txt

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

sbc100 updated this revision to Diff 152505. sbc100 added a comment. - feedback Repository: rC Clang https://reviews.llvm.org/D48443 Files: lib/CodeGen/TargetInfo.cpp test/CodeGen/no-prototype.c Index: test/CodeGen/no-prototype.c

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

sbc100 added a comment. OK, I made this target specific. I think this is cleaner now. Repository: rC Clang https://reviews.llvm.org/D48443 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[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] D41923: [WebAssembly] Remove `-allow-undefined-file wasm.syms` from linker args

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin, jgravelle-google, dschuff, jfb. See: https://github.com/WebAssembly/tool-conventions/issues/35 Repository: rC Clang https://reviews.llvm.org/D41923 Files: lib/Driver/ToolChains/WebAssembly.cpp Index:

[PATCH] D41923: [WebAssembly] Remove `-allow-undefined-file wasm.syms` from linker args

sbc100 updated this revision to Diff 129352. sbc100 added a comment. update tests Repository: rC Clang https://reviews.llvm.org/D41923 Files: lib/Driver/ToolChains/WebAssembly.cpp test/Driver/wasm-toolchain.c Index: test/Driver/wasm-toolchain.c

[PATCH] D41937: [WebAssembly] supports -stdlib=libc++ switch

sbc100 added a comment. lgtm https://reviews.llvm.org/D41937 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41966: [WebAssembly] Add -lc++-abi when linking C++ programs

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin, jgravelle-google, dschuff, jfb. Repository: rC Clang https://reviews.llvm.org/D41966 Files: lib/Driver/ToolChains/WebAssembly.cpp lib/Driver/ToolChains/WebAssembly.h Index:

[PATCH] D41937: [WebAssembly] supports -stdlib=libc++ switch

sbc100 accepted this revision. sbc100 added a comment. This revision is now accepted and ready to land. Nice. I happened to just upload a similar (but orthogonal) patch: https://reviews.llvm.org/D41966 Repository: rC Clang https://reviews.llvm.org/D41937

[PATCH] D41937: [WebAssembly] supports -stdlib=libc++ switch

sbc100 added inline comments. Comment at: lib/Driver/ToolChains/WebAssembly.cpp:151 + case ToolChain::CST_Libcxx: +CmdArgs.push_back("-lc++"); +break; Can you add -lc++abi too? Then I can drop D41966 Repository: rC Clang

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

sbc100 updated this revision to Diff 132098. sbc100 added a comment. Herald added subscribers: cfe-commits, sunfish. - update tests Repository: rC Clang https://reviews.llvm.org/D37831 Files: lib/Driver/ToolChains/CommonArgs.cpp test/Driver/wasm-toolchain.c

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

sbc100 added a comment. After a little discussion about this and the -gc-sections linker flag, the augments for the tools having sensible defaults seem to be winning. This means we don't need the driver to pass these options, which makes our commands lines shorter, and it also means that

[PATCH] D43035: Use 'wasm' over 'wasm32' as default arch name

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin, jgravelle-google, dschuff, jfb. Repository: rC Clang https://reviews.llvm.org/D43035 Files: include/clang/Basic/TargetCXXABI.h lib/Basic/Targets.cpp lib/Basic/Targets/WebAssembly.cpp

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

sbc100 accepted this revision. sbc100 added a comment. This revision is now accepted and ready to land. Might want to line wrap your change description. Repository: rL LLVM https://reviews.llvm.org/D43540 ___ cfe-commits mailing list

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

sbc100 added a comment. Hmm.. actually this is probably going to break the waterfall pretty badly. But maybe we can fix it by re-disabling this explicitly when we build the gcc tests. Repository: rL LLVM https://reviews.llvm.org/D43540 ___

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

sbc100 updated this revision to Diff 152335. sbc100 added a comment. wrong test Repository: rC Clang https://reviews.llvm.org/D48443 Files: lib/CodeGen/CGCall.cpp test/CodeGen/no-prototype.c Index: test/CodeGen/no-prototype.c

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

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin, dschuff. The WebAssembly backend in particular benefits from being able to distinguish between varargs functions (...) and prototype-less C functions. Repository: rC Clang https://reviews.llvm.org/D48443

[PATCH] D49897: [WebAssembly] Force use of lld for test/Driver/wasm-toolchain.c(pp)

sbc100 added a comment. How do you configure clang to use a different linker? It looks like getDefaultLinker() is hardcoded, but maybe I'm missing something. Repository: rC Clang https://reviews.llvm.org/D49897 ___ cfe-commits mailing list

[PATCH] D49825: [CMake] Don't use LIBCXXABI_ENABLE_STATIC option before its declared

sbc100 created this revision. Herald added subscribers: cfe-commits, ldionne, christof, aheejin, mgorny. https://reviews.llvm.org/rL337867 introduced two new cmake_dependent_option options: - LIBCXXABI_INSTALL_STATIC_LIBRARY - LIBCXXABI_INSTALL_SHARED_LIBRARY They depend on

[PATCH] D49573: [CMake] Option to control whether shared/static library is installed

sbc100 added a comment. FYI, this broke the WebAssembly waterfall, where the library is no longer being installed: https://wasm-stat.us/builders/linux/builds/34191 There is probably an easy fix, but I thought I'd post here for the record. Repository: rCXX libc++

[PATCH] D49573: [CMake] Option to control whether shared/static library is installed

sbc100 added a comment. https://reviews.llvm.org/D49825 seems to fix it. Repository: rCXX libc++ https://reviews.llvm.org/D49573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50477: WIP: Ensure that the type of size_t is represended as one of the fixed width types

sbc100 updated this revision to Diff 159794. sbc100 added a comment. remove debugging Repository: rC Clang https://reviews.llvm.org/D50477 Files: lib/Frontend/InitPreprocessor.cpp Index: lib/Frontend/InitPreprocessor.cpp

[PATCH] D50477: WIP: Ensure that the type of size_t is represended as one of the fixed width types

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin. We recently changes `size_t` on wasm from `int` to `long` which had the effect of making the type of size_t (long) not match either int32_t or int64_t. This solution is not very elegant but fixes the build

[PATCH] D50395: [WebAssembly] Remove use of lld -flavor flag

sbc100 updated this revision to Diff 159562. sbc100 added a comment. - update tests Repository: rC Clang https://reviews.llvm.org/D50395 Files: lib/Driver/ToolChains/WebAssembly.cpp lib/Driver/ToolChains/WebAssembly.h test/Driver/wasm-toolchain.c test/Driver/wasm-toolchain.cpp

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

sbc100 added a comment. Should we change int32_t as well? Or does the above code just need fixing? I'm a little concerned because this code is very portable which implies that WebAssembly might be doing something unusual here. Repository: rC Clang https://reviews.llvm.org/D40526

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

sbc100 added a comment. I'm running into an issue with an internal codebase that assumes that size_t is equivalent to either int32_t or int64_t which this change invalidates. After this change we have: size_t -> long int32_t -> int int64_t -> ling long int. e.g.: #include #include

[PATCH] D50477: WIP: Ensure that the type of size_t is represended as one of the fixed width types

sbc100 added a comment. Please ignore this change, I had thought we wanted size_t to match either int32_t or int64_t, but it turns out that isn't a requirement and that code that expects this to be true needs fixing. Repository: rC Clang https://reviews.llvm.org/D50477

[PATCH] D49897: [WebAssembly] Force use of lld for test/Driver/wasm-toolchain.c(pp)

sbc100 accepted this revision. sbc100 added a comment. This revision is now accepted and ready to land. I see. This seems a little strange to me. I'm not sure it makes sense for CLANG_DEFAULT_LINKER to effect targets like WebAssembly that only have single supported linker. I'm OK with

[PATCH] D50395: [WebAssembly] Remove use of lld -flavor flag

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin, jgravelle-google, dschuff. This flag is deprecated. The prefered way to select the lld flavor is by calling it by one of its aliases. Repository: rC Clang https://reviews.llvm.org/D50395 Files:

[PATCH] D50395: [WebAssembly] Remove use of lld -flavor flag

sbc100 updated this revision to Diff 159533. sbc100 added a comment. - update test Repository: rC Clang https://reviews.llvm.org/D50395 Files: lib/Driver/ToolChains/WebAssembly.cpp lib/Driver/ToolChains/WebAssembly.h test/Driver/wasm-toolchain.c test/Driver/wasm-toolchain.cpp

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

sbc100 added a comment. In https://reviews.llvm.org/D40526#1192688, @sunfish wrote: > Is this related to the issue reported in the thread here > ? Ah yes, I'll follow up there. Repository: rC Clang

[PATCH] D49897: [WebAssembly] Force use of lld for test/Driver/wasm-toolchain.c(pp)

sbc100 added a comment. But in the CL description you say "..when configuring clang to use a different linker by default". How is this possible?i.e. do you have a config where these tests are currently failing? Repository: rC Clang https://reviews.llvm.org/D49897

[PATCH] D43681: [WebAssembly] Add exception handling option

sbc100 added a comment. Don't we already have `-fexceptions` and `-fno-exceptions` for this? Repository: rC Clang https://reviews.llvm.org/D43681 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[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] D59743: [WebAssembly] Don't use default GetLinkerPath

sbc100 updated this revision to Diff 192126. sbc100 marked 4 inline comments as done. sbc100 added a comment. - feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59743/new/ https://reviews.llvm.org/D59743 Files:

[PATCH] D59743: [WebAssembly] Don't use default GetLinkerPath

sbc100 updated this revision to Diff 192125. sbc100 marked an inline comment as done. sbc100 added a comment. - feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59743/new/ https://reviews.llvm.org/D59743 Files:

[PATCH] D59743: [WebAssembly] Don't use default GetLinkerPath

sbc100 added inline comments. Comment at: clang/lib/Driver/ToolChains/Fuchsia.h:89 - const char *getDefaultLinker() const override { -return "ld.lld"; - } + const char *getDefaultLinker() const override { return "ld.lld"; } phosek wrote: > This seems

[PATCH] D59721: [WebAssembly] Make driver -pthread imply linker --shared-memory

sbc100 added a comment. Hi @phosek ! Fixed in rL356847 . This did make me look a little deeper into this issue though, and wonder if the CLANG_DEFAULT_LINKER is implemented in a useful way right now. I believe the reason this came up for you guys in that you

[PATCH] D59743: [WebAssembly] Don't use default GetLinkerPath

sbc100 created this revision. Herald added subscribers: cfe-commits, atanasyan, jrtc27, sunfish, aheejin, jgravelle-google, sdardis, dschuff. Herald added a project: clang. We can't (don't want to) honor the same set of "-fuse-ld" flags with WebAssembly since the ELF linkers (ld.lld, ld.gnu,

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

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

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

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

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

sbc100 added a comment. I think I like this. We may find a reason down the line to allow passive to be set per-segment, but now keeping it simple and lettings the linker decide makes sense. Comment at: llvm/test/CodeGen/WebAssembly/atomic-mem-consistency.ll:1 -; RUN: not llc

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

sbc100 added a comment. I certainly like that llc now does the right thing by default! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58742/new/ https://reviews.llvm.org/D58742 ___ cfe-commits mailing

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

sbc100 added a comment. Yes it makes sense to me to set `-mthread-model=posix` when `-pthread` is passed on the commandline. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57874/new/ https://reviews.llvm.org/D57874

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

sbc100 added inline comments. Comment at: lib/Driver/ToolChains/WebAssembly.cpp:66 +if (Args.hasFlag(clang::driver::options::OPT_pthread, + clang::driver::options::OPT_no_pthread), +false) aheejin wrote: > This code is not

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

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

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

sbc100 added inline comments. Comment at: lib/Driver/ToolChains/WebAssembly.cpp:66 +if (Args.hasFlag(clang::driver::options::OPT_pthread, + clang::driver::options::OPT_no_pthread), +false) aheejin wrote: > tlively wrote: > >

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

sbc100 added inline comments. Comment at: lib/Driver/ToolChains/WebAssembly.cpp:50 + bool HasNoPthread = + !Pthread && DriverArgs.hasArg(clang::driver::options::OPT_no_pthread); + tlively wrote: > Should this logic use `getLastArg` or perhaps

[PATCH] D57798: [WebAssembly] Add atomics target option

sbc100 added a comment. How does this relate to the existing ` -mattr=+atomics` of llc? Will it also result in `attributes #0 = { "target-features"="+atomics" }` in the bitcode? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57798/new/

[PATCH] D57345: Make clang/test/Index/pch-from-libclang.c pass in more places

sbc100 added a comment. Herald added a project: LLVM. I think this broke the BUILD_SHARED_LIBS build: https://reviews.llvm.org/D57556 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57345/new/ https://reviews.llvm.org/D57345

[PATCH] D62047: [WebAssembly] Add multivalue and tail-call target features

sbc100 added a comment. Does it make sense to support the flags before we support the feature? Otherwise lgtm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62047/new/ https://reviews.llvm.org/D62047

[PATCH] D62709: Fix -DBUILD_SHARED_LIBS=ON build after rL362160

sbc100 created this revision. Herald added subscribers: llvm-commits, cfe-commits, dang, dexonsmith, steven_wu, aheejin, hiraditya, mgorny, mehdi_amini. Herald added projects: clang, LLVM. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D62709 Files:

[PATCH] D62915: git-clang-format: Remove trailing whitespace in docstring. NFC.

sbc100 created this revision. Herald added subscribers: cfe-commits, aheejin. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D62915 Files: clang/tools/clang-format/git-clang-format Index: clang/tools/clang-format/git-clang-format

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

sbc100 updated this revision to Diff 203634. sbc100 added a comment. - more OS Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63030/new/ https://reviews.llvm.org/D63030 Files: clang/lib/Driver/ToolChains/WebAssembly.cpp

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

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin, jgravelle-google, dschuff. Herald added a project: clang. Move include path construction from InitHeaderSearch::AddDefaultIncludePaths in the Driver which appears to be the more modern/correct way of doing

[PATCH] D63245: clang-scan-deps: Fix -DBUILD_SHARED_LIBS=ON build

sbc100 created this revision. Herald added subscribers: cfe-commits, tschuett, aheejin, mgorny. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D63245 Files: clang/tools/clang-scan-deps/CMakeLists.txt Index:

[PATCH] D63742: [WebAssembly] Implement Address Sanitizer for Emscripten

sbc100 added a comment. I wonder if we should use the linux/unix convention or `edata` `etext` and `end`? Terrible names obviously but there is precedent. I can't remember why I didn't do that for __data_end and __heap_base. If not, then perhaps this should be called __data_start to match

[PATCH] D63742: [WebAssembly] Implement Address Sanitizer for Emscripten

sbc100 added inline comments. Comment at: lld/wasm/Writer.cpp:228 + if (WasmSym::GlobalBase) +WasmSym::GlobalBase->setVirtualAddress(Config->GlobalBase); + Surly if emscripten is passing in --global-base it already knows this value? Otherwise lgtm.

[PATCH] D63081: [WebAssembly] Cleanup toolchain test files. NFC.

sbc100 created this revision. Herald added subscribers: cfe-commits, jfb, sunfish, aheejin, jgravelle-google, dschuff. Herald added a project: clang. sbc100 added a reviewer: dschuff. Herald added a subscriber: ormris. Split up long lines to improve test readability. Repository: rG LLVM

[PATCH] D62320: Fix LLVM_LINK_LLVM_DYLIB build after rC361285

sbc100 created this revision. Herald added subscribers: cfe-commits, aheejin, mgorny. Herald added a project: clang. All the other unittests I can find specify LLVMTestingSupport in target_link_libraries not as part of LLVM_LINK_COMPONENTS. I'm how/why this fixes the build issue, but its

[PATCH] D62201: [LibTooling] Address post-commit feedback for r361152

sbc100 added a comment. I think this build broke the `-DLLVM_BUILD_LLVM_DYLIB=ON -DLLVM_LINK_LLVM_DYLIB=ON` build. e.g https://ci.chromium.org/p/wasm/builders/ci/linux/6457 FAILED: tools/clang/unittests/Tooling/ToolingTests : &&

[PATCH] D62320: Fix LLVM_LINK_LLVM_DYLIB build after rC361285

sbc100 abandoned this revision. sbc100 added a comment. This was fixed in https://reviews.llvm.org/D62333 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62320/new/ https://reviews.llvm.org/D62320 ___

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

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin, jgravelle-google, dschuff. Herald added a project: clang. sbc100 added a reviewer: dschuff. This is in line with other platforms. Also, move the single statement methods into the header (also in line with

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

sbc100 updated this revision to Diff 197821. sbc100 added a comment. Herald added a subscriber: ormris. tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61452/new/ https://reviews.llvm.org/D61452 Files:

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

sbc100 added a comment. Even in the case of wasm64 it might well be that somebody wants to build two different sysroots (as they would be arm and arm64 on linux). Even if we don't choose to ship a single arch sysroot for wasi somebody else might. This doesn't seem like it does any harm to

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

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

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

sbc100 updated this revision to Diff 197823. sbc100 added a comment. - update comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61452/new/ https://reviews.llvm.org/D61452 Files: clang/lib/Driver/ToolChains/WebAssembly.cpp

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

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin, jgravelle-google, dschuff. Herald added a project: clang. Even though the toolchain supports multiarch, we still want to be able to support single-arch sysroots. This also helps for the case where libraries

[PATCH] D63742: [WebAssembly] Implement Address Sanitizer for Emscripten

sbc100 accepted this revision. sbc100 added a comment. This revision is now accepted and ready to land. Remember to remove "A symbol __global_base is added so that code may know where the shadow memory ends and real memory begins." from the CL description. Repository: rG LLVM Github Monorepo

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

sbc100 added a comment. I wonder if `__tls_base` should be allocated by the embedder (or by the parent/creator thread). Then it could be an *immutable* global import that is allocated up front. I guess `__stack_pointer` should be treated in the same way (except mutable). I don't want to

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

sbc100 added a comment. Its really great to see this change BTW! Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D64537 ___ cfe-commits mailing list

[PATCH] D67208: [WebAssembly] Add -fwasm-exceptions for wasm EH

sbc100 added a comment. Is there some reason why clang can't pass the -exception-model=wasm flag programatically to llc when -mmexception-handling is set? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67208/new/ https://reviews.llvm.org/D67208

[PATCH] D67208: [WebAssembly] Add -fwasm-exceptions for wasm EH

sbc100 added a comment. But why not make -mexception-handling be the option that enabled everything? I mean -mexception-handling is a flag we have today.. if you add -fwasm-exceptions what does -mexception-handling meaning? Is it still useful? Repository: rG LLVM Github Monorepo

[PATCH] D67208: [WebAssembly] Add -fwasm-exceptions for wasm EH

sbc100 added a comment. I don't really see why we can't just have the existing `-mexception-handling` take on this new meaning? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67208/new/ https://reviews.llvm.org/D67208

[PATCH] D67208: [WebAssembly] Add -fwasm-exceptions for wasm EH

sbc100 added a comment. Derek, are you saying that `-mexception-handling` is somehow related to `-fno-exceptions`. What is the relationship?I wasn't aware this change was related to `-fno-exceptions`. `-pthreads` enabling `-matomics` and `-mbulk-memory`make some sense because each of

[PATCH] D65684: [WebAssembly] Lower ASan constructor priority on Emscripten

sbc100 added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:132 static const uint64_t kAsanCtorAndDtorPriority = 1; +static const uint64_t kAsanEmscriptenCtorAndDtorPriority = 50; static const char *const kAsanReportErrorTemplate =

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

sbc100 added a comment. In D64537#1586614 , @quantum wrote: > In D64537#1586577 , @dschuff wrote: > > > Another high-level question (based just on reading the CL description): The > > TLS-size intrinsic is

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

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

[PATCH] D64955: Fix formatting of inline argument comments. NFC.

sbc100 created this revision. Herald added subscribers: cfe-commits, aheejin, dschuff. Herald added a project: clang. Also, remove the final arg from ItaniumCXXABI in the PNaCl case since its not needed. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D64955 Files:

[PATCH] D64957: [WebAssembly] Don't set UseARMGuardVarABI, just use the default (64-bit)

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin, kristof.beyls, jgravelle-google, javed.absar, dschuff. Herald added a project: clang. sbc100 added a reviewer: sunfish. This fixes a disagreement between libc++abi and clang about the width of the guard

[PATCH] D64957: [WebAssembly] Don't set UseARMGuardVarABI, just use the default (64-bit)

sbc100 abandoned this revision. sbc100 added a comment. We are going to change libc++abi instead: https://reviews.llvm.org/D64961 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64957/new/ https://reviews.llvm.org/D64957

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

sbc100 accepted this revision. sbc100 added inline comments. This revision is now accepted and ready to land. Comment at: lld/wasm/Writer.cpp:400 + StringRef name = segment->getName(); + if (name.startswith(".tdata.") || name.startswith(".tbss.")) +tlsUsed =

[PATCH] D65028: [WebAssembly] Compute and export TLS block alignment

sbc100 added inline comments. Comment at: lld/trunk/wasm/Symbols.h:438 + // __tls_size + // Symbol whose value is the alignment of the TLS block. __tls_align Comment at: lld/trunk/wasm/Symbols.h:439 + // __tls_size + // Symbol whose

[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

[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=232660#toc Repository: rG

  1   2   3   4   >