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

[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: lib/Driv

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

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

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

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

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

[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 lib/Driver/ToolChains/WebAssembly.c

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

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

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

[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 lib/CodeGen/CGB

[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 cfe-commits@

[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 ___ 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] 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 http://lists.llvm.org/cgi-bin/mailman/listin

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

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

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

[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: [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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

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

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

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

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

[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, etc)

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

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

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

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

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

[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 clang/test/Driver

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

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

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

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

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

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

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

[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: clang/lib/CodeGen/CMakeList

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

[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 clang/lib/Frontend/InitH

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

[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: clang/tools/clang-scan-deps/CMakeLists

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

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

[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 = tru

[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 per-func

[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. > > I'

[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: clang/lib/C

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

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

[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 = "__asan

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

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

[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] 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] 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 : && /b/swarming/w/ir/cache/builder/src/src/work/v8/v8/third_part

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

[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 ___ cf

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

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

[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: test/libcxx/utilities/any/size

[PATCH] D80876: [clang] Default to windows response files when running on windows

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin. Herald added a project: clang. sbc100 added a reviewer: ruiu. This matches other tools such as llvm-ar, lld, etc. This consistency is useful when authoring downstream tools as it means they can always use windo

[PATCH] D80876: [clang] Default to windows response files when running on windows

sbc100 marked an inline comment as done. sbc100 added inline comments. Comment at: clang/tools/driver/driver.cpp:391 +if (ClangCLMode || +llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows()) + Tokenizer = &llvm::cl::TokenizeWindowsCommandLine; -

[PATCH] D80876: [clang] Default to windows response files when running on windows

sbc100 added a comment. If we want want to have the default on windows be dependent on mingw vs not-mingw then we should do it across the board so it applies to llvm-ar, lld, and other tools. Right now I don't think any of those other tools have any special mingw handling. Would it make sense

[PATCH] D80876: [clang] Default to windows response files when running on windows

sbc100 added a comment. The problem I ran into regarding the quoting style differences was trying to refer to filenames that contains single quote characters: Here is our test case that I've currently disabled on windows pending the resolution of this issue: https://github.com/emscripten-core/e

[PATCH] D80876: [clang] Default to windows response files when running on windows

sbc100 added a comment. Do you know how gcc handles this case when running on mingw? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80876/new/ https://reviews.llvm.org/D80876 ___ cfe-commits mailing lis

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

sbc100 created this revision. Herald added subscribers: cfe-commits, jfb, arphaman, sunfish, aheejin, hiraditya, jgravelle-google, dschuff. Herald added a project: clang. Since this feature is now merged into the upstream wasm spec it makes sense to enable it by default, at least for the `default

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

sbc100 added inline comments. Comment at: clang/lib/Basic/Targets/WebAssembly.h:33 - bool HasNontrappingFPToInt = false; + bool HasNontrappingFPToInt = true; bool HasSignExt = false; tlively wrote: > It seems strange to change the default here. x86 initial

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

sbc100 updated this revision to Diff 256712. sbc100 marked 7 inline comments as done. sbc100 added a comment. feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77908/new/ https://reviews.llvm.org/D77908 Files: clang/lib/Basic/Targets/WebAss

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

sbc100 updated this revision to Diff 256713. sbc100 added a comment. err.. arc diff did something funny. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77908/new/ https://reviews.llvm.org/D77908 Files: clang/lib/Basic/Targets/WebAssembly.cpp cl

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

sbc100 added a comment. I don't have documented plan anywhere. I simply observed that we have CPUs called `mvp` and `generic` and it made sense to start including more things in the generic CPU, leaving the `mvp` option open to those who want to be conservative. I don't think its a huge issue

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG3ea1c62cbae2: [WebAssembly] Emit .llvmcmd and .llvmbc as custom sections (authored by sbc100). Changed prior to commit: https://reviews.llvm.org/D77115?vs=253941&id=257476#toc Repository: rG LLVM Git

[PATCH] D80876: [clang] Default to windows response files when running on windows

sbc100 added a comment. In D80876#2070233 , @mstorsjo wrote: > In D80876#2069970 , @sbc100 wrote: > > > Do you know how gcc handles this case when running on mingw? > > > It uses the gnu/unix quoting style - so that

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

sbc100 accepted this revision. sbc100 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Basic/Targets/WebAssembly.h:139 +// Emscripten targets. +return getTriple().isOSEmscripten(); + } I'm not sure we need thi

[PATCH] D83713: [WebAssembly] Triple::wasm64 related cleanup

sbc100 accepted this revision. sbc100 added inline comments. This revision is now accepted and ready to land. Comment at: lld/wasm/InputFiles.cpp:583 } + config->is64 = t.getArch() == Triple::wasm64; std::vector keptComdats; Should we error out here if you

[PATCH] D82130: [WebAssembly] Adding 64-bit versions of __stack_pointer and other globals

sbc100 accepted this revision. sbc100 added a comment. This revision is now accepted and ready to land. Great! Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:68 + else +CmdArgs.push_back("-m wasm32"); + All the other drivers seem to do `CmdArgs.pu

[PATCH] D82346: [WebAssebmly] Fully disable 'protected' visibility

sbc100 created this revision. Herald added subscribers: cfe-commits, aheejin, jgravelle-google, dschuff. Herald added a project: clang. sbc100 added a reviewer: dschuff. Emscripten doesn't use protected visibility either. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D82346 Fi

[PATCH] D82346: [WebAssebmly] Fully disable 'protected' visibility

This revision was automatically updated to reflect the committed changes. Closed by commit rG5804a8b1228b: [WebAssebmly] Fully disable 'protected' visibility (authored by sbc100). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82346/new/ https://revi

[PATCH] D82130: [WebAssembly] Adding 64-bit versions of __stack_pointer and other globals

sbc100 accepted this revision. sbc100 added a comment. Linker changes still lgtm % comments Comment at: lld/wasm/InputChunks.cpp:338 + auto WASM_OPCODE_PTR_CONST = + config->is64 ? WASM_OPCODE_I64_CONST : WASM_OPCODE_I32_CONST; sbc100 wrote: > Just use

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

sbc100 added a comment. In D70500#134 , @bernhard wrote: > > Would it work to increase the memory size, and then put your data in the > > new space this creates at the end of memory? > > > > > `__data_end` and `__heap_base` aren't things you can move

[PATCH] D79534: [clang][WebAssembly] Only expose wait and notify builtins with atomics

sbc100 accepted this revision. sbc100 added a comment. This revision is now accepted and ready to land. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79534/new/ https://reviews.llvm.org/D79534 ___ cfe-co

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

  1   2   3   4   >