[PATCH] D43630: [Driver] Fix search paths on x32

dschuff added a comment. This should probably have some tests. It looks like there are some existing x32 tests in test/Driver/linux-header-search.cpp and test/Driver/cross-linux.c that might be relevant for updating. Repository: rC Clang https://reviews.llvm.org/D43630 __

[PATCH] D44931: [WebAssembly] Use Windows EH instructions for Wasm EH

dschuff accepted this revision. dschuff added a comment. This revision is now accepted and ready to land. LGTM assuming we are convinced for now that finding the rethrow block from the CatchSwitch will work. Does this need to wait until after https://reviews.llvm.org/D43746 or other CLs from tha

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

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

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

dschuff accepted this revision. dschuff added inline comments. This revision is now accepted and ready to land. Comment at: lib/Driver/ToolChain.cpp:318 + else +OSLibName = getOS(); llvm::sys::path::append(Path, "lib", OSLibName); sbc100 wrote: > dschuff

[PATCH] D49396: [WebAssembly] Support for atomic.wait / atomic.wake builtins

dschuff added inline comments. Comment at: include/clang/Basic/BuiltinsWebAssembly.def:38 +// Atomic wait and wake. +BUILTIN(__builtin_wasm_atomic_wait_i32, "Uii*iLLi", "n") +BUILTIN(__builtin_wasm_atomic_wait_i64, "UiLLi*LLiLLi", "n") So this means that the sign

[PATCH] D49396: [WebAssembly] Support for atomic.wait / atomic.wake builtins

dschuff accepted this revision. dschuff added inline comments. This revision is now accepted and ready to land. Comment at: include/clang/Basic/BuiltinsWebAssembly.def:38 +// Atomic wait and wake. +BUILTIN(__builtin_wasm_atomic_wait_i32, "Uii*iLLi", "n") +BUILTIN(__builtin_wasm_a

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

dschuff accepted this revision. dschuff added a comment. This revision is now accepted and ready to land. The code LGTM. If there are objections to the scheme overall they can go in tool-conventions. Repository: rC Clang https://reviews.llvm.org/D41923

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

dschuff added inline comments. Comment at: lib/Basic/Targets/WebAssembly.h:108 IntType getLeastIntTypeByWidth(unsigned BitWidth, bool IsSigned) const final { -// WebAssembly uses long long for int_least64_t and int_fast64_t. -return BitWidth == 64 +// WebAssembly

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

dschuff added inline comments. Comment at: lib/Basic/Targets/WebAssembly.h:108 IntType getLeastIntTypeByWidth(unsigned BitWidth, bool IsSigned) const final { -// WebAssembly uses long long for int_least64_t and int_fast64_t. -return BitWidth == 64 +// WebAssembly

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

dschuff added inline comments. Comment at: lib/Basic/Targets/WebAssembly.h:108 IntType getLeastIntTypeByWidth(unsigned BitWidth, bool IsSigned) const final { -// WebAssembly uses long long for int_least64_t and int_fast64_t. -return BitWidth == 64 +// WebAssembly

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

dschuff accepted this revision. dschuff added a comment. This revision is now accepted and ready to land. This flag turns on CPU features (i.e. there's one for each new wasm feature proposal that has to be feature-detected), so I think this makes sense to be consistent with all the others. I cou

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

dschuff added inline comments. Comment at: lib/CodeGen/CGCall.cpp:1850 +FuncAttrs.addAttribute("no-prototype"); AddAttributesFromFunctionProtoType( getContext(), FuncAttrs, Fn->getType()->getAs()); Would it make sense to put this in `Add

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

dschuff added inline comments. Comment at: lib/CodeGen/CGCall.cpp:1849 + if (!AttrOnCallSite && !Fn->hasPrototype()) +FuncAttrs.addAttribute("no-prototype"); AddAttributesFromFunctionProtoType( aheejin wrote: > Is there a reason why this is no

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

dschuff added inline comments. Comment at: lib/CodeGen/CGCall.cpp:1849 + if (!AttrOnCallSite && !Fn->hasPrototype()) +FuncAttrs.addAttribute("no-prototype"); AddAttributesFromFunctionProtoType( dschuff wrote: > aheejin wrote: > > Is there a re

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

dschuff accepted this revision. dschuff added a comment. This revision is now accepted and ready to land. LGTM; don't forget to include all the context in the future Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57155/new/ https://reviews.llvm.org/D57155 __

[PATCH] D59448: [WebAssembly] Change wasm.throw's first argument to an immediate

dschuff accepted this revision. dschuff added a comment. This revision is now accepted and ready to land. LGTM; I wonder if it makes sense to have predefined macro for the C++ tag (or perhaps just a regular macro for use in libcxxabi?) Repository: rC Clang CHANGES SINCE LAST ACTION https:/

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

dschuff added a comment. Reverted in rG039be787914610c28cba45c4557454e0a96939ab . Caused a strange error with the waterfall sysroot's build of libcxx: https://logs.chromium.org/logs/wasm/buildbucket/cr-buildbucket.appspot.com/

[PATCH] D52856: [WebAssembly] __builtin_wasm_replace_lane_* builtins

dschuff added a comment. W is int64_t (which is long long for wasm32 and would probably be long for wasm64 since that would probably LP64 (like Linux) rather than LLP64 (Like Win64). So if we want it to be the same for both wasm32 and wasm64, I guess we want LL. Repository: rC Clang https:

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

dschuff added a comment. So this CL has the effect that setting `-pthreads` will also set `-matomics`. Currently as you mentioned we have the problem that we can't make our current logic of "do we lower away the atomics" be controlled by the target features because it's done at pass config time

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

dschuff added a comment. Oh I guess another option would just be to pin all 3 flags together here, but since `-pthread` sets a preprocessor define and may also affect linker behavior, I think it's fine to allow atomic codegen without setting `-pthread`. Repository: rC Clang CHANGES SINCE LA

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

dschuff added a comment. LGTM Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63081/new/ https://reviews.llvm.org/D63081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/lis

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

dschuff added a comment. If there's any chance this TLS ABI could be useful for WASI (I don't know if there's been any WASI work on threads yet, but it seems like there's no reason it couldn't be), then we should start a doc in tool-conventions for it. If not then we should get it behind the em

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

dschuff added a comment. Oh, btw, any reason these have to be passive segments? Why can't we just make them active segments and let the VM initialize them for us? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D6

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

dschuff added a comment. The `offset` field of a segment can be a constant expression which can be a `global.get` of an imported global. So we could have an imported global `__tls_base` which is different for

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

dschuff added a comment. Another high-level question (based just on reading the CL description): The TLS-size intrinsic is per-function, does that mean that the tls-init function is called for every function? are there just multiple TLS sections per object file? Repository: rG LLVM Github M

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

dschuff added a comment. I had a reply that got eaten here, so I'm going to keep trolling you on your CL since we don't have a design doc for this. The `offset` field of a data segment initializer can be a `global.get` on an imported global. (https://webassembly.github.io/spec/core/valid/instr

[PATCH] D66168: [WebAssembly] Make clang emit correct va_arg code for structs

dschuff accepted this revision. dschuff added a comment. This revision is now accepted and ready to land. LGTM. Nice that we had a test which tested the wrong thing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66168/new/ https://reviews.llvm.org/

[PATCH] D66255: [WebAssembly] Correctly handle va_arg of zero-sized structures

dschuff added inline comments. Comment at: clang/test/CodeGen/wasm-varargs.c:104 + +struct S test_zero_size_struct(char *fmt, ...) { + va_list va; This should maybe be called "test_empty_struct" since its size is actually 1 and not zero. Comm

[PATCH] D69075: [WebAssembly] -pthread implies -target-feature +sign-ext

dschuff accepted this revision. dschuff added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Driver/wasm-toolchain.c:76 +// '-pthread' not allowed with '-mno-mutable-globals' +// RUN: %clang -### -no-canonical-prefixes -target wasm32-un

[PATCH] D80362: [WebAssembly] Warn on exception spec only when Wasm EH is used

dschuff accepted this revision. dschuff added a comment. This revision is now accepted and ready to land. otherwise LGTM Comment at: clang/docs/DiagnosticsReference.rst:14018 ++-

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

dschuff added a comment. Yeah I think a 64-bit version of userstack.ll makes sense. (a fork or a second set of CHECKs, whatever you think would be cleaner). There's also `stack-alignment.ll` which covers dynamic stack adjustment. Comment at: lld/wasm/Driver.cpp:385 +Stri

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

dschuff added a comment. So the code LGTM, were you going to add to usertest.ll in this CL? Comment at: llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp:84 WasmSym->setGlobalType(wasm::WasmGlobalType{ -uint8_t(Subtarget.hasAddr64() ? wasm::WASM_TYPE_I64 -

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

dschuff accepted this revision. dschuff added a comment. Compiler changes LGTM Comment at: llvm/test/CodeGen/WebAssembly/stack-alignment.ll:1 -; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers | FileCheck %s - -target datalayout = "e-

[PATCH] D78441: Delete NaCl support

dschuff added a comment. @jfb thanks for the heads-up. I replied on the mailing list thread. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78441/new/ https://reviews.llvm.org/D78441 ___ cfe-commits mai

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

dschuff added a comment. We can definitely still talk about what you are trying to do, and it would probably be useful to have more folks involved. Opening an issue on https://github.com/WebAssembly/tool-conventions/ might be useful since it involves the conventions that LLVM and clang use. If

[PATCH] D85685: [WIP] Support dwarf fission for wasm object files

dschuff created this revision. dschuff added reviewers: sbc100, aardappel. Herald added subscribers: llvm-commits, cfe-commits, sunfish, hiraditya, aprantl. Herald added projects: clang, LLVM. dschuff requested review of this revision. Herald added a subscriber: aheejin. Initial support for dwarf

[PATCH] D85685: [WIP] Support dwarf fission for wasm object files

dschuff updated this revision to Diff 284522. dschuff added a comment. Fix bad diff Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85685/new/ https://reviews.llvm.org/D85685 Files: clang/lib/Driver/ToolChains/Clang.cpp llvm/include/llvm/MC/MCWa

[PATCH] D79655: [WebAssembly] Ignore exception specifications

dschuff added a comment. Actually, would it be possible to not ignore `throw()` but make it an alias for `noexcept(true)`? Apparently that is the standard behavior in C++17, so it might make more sense to just implement that now rather than just warning all the time and ignoring it. It would al

[PATCH] D79655: [WebAssembly] Handle exception specifications

dschuff accepted this revision. dschuff added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/CodeGenCXX/wasm-eh.cpp:399 +// RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions -fexceptions -fcxx-exceptions -fwasm-exceptio

[PATCH] D149917: [lld][WebAssembly] Add --keep-section flag

dschuff added inline comments. Comment at: lld/wasm/Options.td:196 +defm keep_section: Eq<"keep-section", + "Preserve a section even when --strip-all is given. This is useful for compiler drivers such as clang or emcc that, for example, depend on the features section for

[PATCH] D149917: [lld][WebAssembly] Add --keep-section flag

dschuff accepted this revision. dschuff added a comment. otherwise LGTM Comment at: lld/wasm/Options.td:196 +defm keep_section: Eq<"keep-section", + "Preserve a section even when --strip-all is given. This is useful for compiler drivers such as clang or emcc that, for e

[PATCH] D118573: [clang][WebAssemmbly] Call TargetInfo::adjust in derived method.

dschuff accepted this revision. dschuff added a comment. This revision is now accepted and ready to land. Looks OK to me. All of these options handle explicit overrides to the ABI which I guess just didn't work before but doesn't change any defaults. We should probably put something in the emscr

[PATCH] D123763: [randstruct] Enforce using a designated init for a randomized struct

dschuff added a comment. In D123763#3485836 , @sbc100 wrote: > This new test has been failing on the emscripten builders.. seemingly ever > since it landed: This uses a fairly old sysroot. I tried with a new version of libcxx, and it seems to be passin

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

dschuff added a comment. I think the expectation is that if you use an intrinsics header that has an intrinsic for each machine instruction, that each intrinsic call should result in exactly one machine instruction with the same arguments you passed to it. If the vector operation is created som

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

dschuff added a comment. Oh, interesting I didn't notice that those are implementations of the target-specific intrinsics. I wonder if they do that so they can implement the intrinsics on hardware that doesn't support the corresponding instruction? In a situation other than that I think I'd be

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

dschuff added a comment. If this is to be like `-fdwarf-exceptions` I assume the idea is that eventually it will default on when targeting wasm, and become one of those flags that most users never set. In that case it really just selects the default exception model, and thus it should be compat

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

dschuff added a comment. Oh, you're right, I'm conflating `-mexception-handling` with `-fexceptions`. And also `-fno-exceptions` with `noexcept`. So... just forget I was here Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D85685: [WIP] Support dwarf fission for wasm object files

dschuff updated this revision to Diff 292055. dschuff added a comment. Get the right sections in the objects, add tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85685/new/ https://reviews.llvm.org/D85685 Files: llvm/lib/MC/WasmObjectWriter.

[PATCH] D85685: [WIP] Support dwarf fission for wasm object files

dschuff updated this revision to Diff 292056. dschuff added a comment. Herald added subscribers: sstefan1, ormris, jgravelle-google. Herald added a reviewer: jdoerfert. upload the correct diff Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85685/new/

[PATCH] D85685: Support dwarf fission for wasm object files

dschuff added a comment. OK, I think this is ready to review for real. Can you take another look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85685/new/ https://reviews.llvm.org/D85685 ___ cfe-commits

[PATCH] D85685: Support dwarf fission for wasm object files

dschuff added a comment. In D85685#2275585 , @sbc100 wrote: > Seems reasonable. Do you think this way is cleaner than the way elf does > it? Looks like ELF creates two different ELFWriter inside the > ELFDwoObjectWriter subclass right? Yeah, ELF spl

[PATCH] D85685: Support dwarf fission for wasm object files

dschuff added a comment. In D85685#2277998 , @dschuff wrote: > > Yeah, ELF splits ELFWriter out from ELFObjectWriter, and then instantiates it > twice. It's all because ELFObjectWriter has to derive from MCObjectWriter > which was clearly not designe

[PATCH] D85685: Support dwarf fission for wasm object files

dschuff updated this revision to Diff 292624. dschuff added a comment. rebase, address comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85685/new/ https://reviews.llvm.org/D85685 Files: llvm/lib/MC/WasmObjectWriter.cpp Index: llvm/lib/MC/

[PATCH] D85685: Support dwarf fission for wasm object files

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0ff28fa6a756: Support dwarf fission for wasm object files (authored by dschuff). Changed prior to commit: https://reviews.llvm.org/D85685?vs=29262

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

dschuff created this revision. dschuff added reviewers: aardappel, sbc100. Herald added subscribers: llvm-commits, cfe-commits, ecnelises, sunfish, hiraditya, jgravelle-google. Herald added projects: clang, LLVM. dschuff requested review of this revision. Herald added subscribers: ormris, aheejin.

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

dschuff added inline comments. Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962 + case Triple::Wasm: +return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash), + ~0); I may add a couple more tests to this, but I

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

dschuff added inline comments. Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962 + case Triple::Wasm: +return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash), + ~0); dschuff wrote: > I may add a couple more tes

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

dschuff added inline comments. Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962 + case Triple::Wasm: +return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash), + ~0); dblaikie wrote: > dschuff wrote: > > dschuff

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

dschuff added inline comments. Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962 + case Triple::Wasm: +return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash), + ~0); dblaikie wrote: > dschuff wrote: > > dblaiki

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

dschuff updated this revision to Diff 301133. dschuff added a comment. use GenericSectionID instead of ~0 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88603/new/ https://reviews.llvm.org/D88603 Files: llvm/lib/MC/MCObjectFileInfo.cpp Index: l

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbcb8a119df21: [WebAssembly] Add support for DWARF type units (authored by dschuff). Changed prior to commit: https://reviews.llvm.org/D88603?vs=30

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

dschuff added a comment. This broke the bots for some strange reason that didn't reproduce locally. But because it was ~all of them, I probably just did something stupid. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88603/new/ https://reviews.llv

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

dschuff added a comment. In D88603#2359554 , @dblaikie wrote: > In D88603#2357973 , @dschuff wrote: > >> This broke the bots for some strange reason that didn't reproduce locally. >> But because it was ~all of them

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

dschuff updated this revision to Diff 301486. dschuff added a comment. use getOrCreate Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88603/new/ https://reviews.llvm.org/D88603 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/debu

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

dschuff updated this revision to Diff 301488. dschuff added a comment. can't just getOrCreate symbol without dealing with the section just disable the test for now as we don't need dw5 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88603/new/ https:

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

dschuff updated this revision to Diff 301490. dschuff added a comment. fix diff; it should be against master Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88603/new/ https://reviews.llvm.org/D88603 Files: clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

dschuff added a comment. @sbc100 I found that the cause of the assertion is that in dwarf 5, the type units apparently go in the .debug_info section (instead of the .debug_type section), and this section already exists (but it was created as a non-comdat). So when `getWasmSection` tries to look

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

dschuff added a comment. Can you upload the diff with full context (e.g. use `diff -U ` or use arcanist to upload)? I'm a bit confused; the commit message talks about X32 being a separate architecture, but you're not adding any new architecture triples here (it still uses x86_64 as the arc

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

dschuff added a comment. One other question then: do you know if Debian and/or Ubuntu still have the same support for running x32 programs on the regular x86-64 distribution? (presumably yes, since you aren't changing the existing behavior). AFAIK clang's current support was developed against Ub

[PATCH] D102791: [WebAssembly] Warn on exception spec for Emscripten EH

dschuff added a comment. BTW Is there a way to disable this warning? Since IIUC this could cause code that was not warning (because it used -fno-exceptions or used emscripten's default of -fignore-exceptions) to now start warning, sometimes that makes users who use -Werror unhappy. ==

[PATCH] D102791: [WebAssembly] Warn on exception spec for Emscripten EH

dschuff accepted this revision. dschuff added a comment. Got it, this makes sense to me too. And since it can be turned off, I'm not too worried about annoying users. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102791/new/ https://reviews.llvm.o

[PATCH] D107613: [Clang][DiagnosticSemaKinds] combine diagnostic texts

dschuff added a comment. It looks like this error is intended to catch mismatches for attributes that can affect codegen such as noreturn (in which case it makes sense to have it as an error) but it also now fires for cases such as `__attribute__(warning())` which often do not duplicate the att

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

dschuff created this revision. dschuff added a reviewer: azakai. Herald added subscribers: sunfish, hiraditya, jgravelle-google, sbc100. dschuff requested review of this revision. Herald added subscribers: cfe-commits, aheejin. Herald added projects: clang, LLVM. This fix goes along with d1a96e906

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

dschuff added a reviewer: sbc100. dschuff added a comment. This fix should go in ASAP (or else we should revert d1a96e906cc03a95cfd41a1f22bdda92651250c7 ) to fix the mismatch between LLVM and clang. Repository: rG LLVM Git

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

dschuff updated this revision to Diff 357665. dschuff added a comment. - fix wasm64 copypasta Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105749/new/ https://reviews.llvm.org/D105749 Files: clang/lib/Basic/Targets/WebAssembly.h llvm/lib/Targ

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

dschuff added inline comments. Comment at: clang/lib/Basic/Targets/WebAssembly.h:175 +if (T.isOSEmscripten()) + resetDataLayout("e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1"); +else kripken wrote: > Should this not be > > resetDataLayout("e-m:e-p:

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGac02baab48c2: WebAssembly: Update datalayout to match fp128 ABI change (authored by dschuff). Repository: rG LLVM Github Monorepo CHANGES SINCE L

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

dschuff added inline comments. Comment at: llvm/test/CodeGen/WebAssembly/varargs.ll:5 -target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128" -target triple = "wasm32-unknown-unknown" +target triple = "wasm32-unknown-emscripten" sunfish wrote: > It appears th

[PATCH] D97834: [WebAssembly] Disable uses of __clang_call_terminate

dschuff accepted this revision. dschuff added a comment. I agree this is a good approach, and I also like that it's smaller and simpler. In the future we can revisit whether following this particular Itanium convention buys us anything useful or not. Repository: rG LLVM Github Monorepo CHAN

[PATCH] D98457: [WebAssembly] Remove unimplemented-simd target features

dschuff accepted this revision. dschuff added inline comments. This revision is now accepted and ready to land. Comment at: clang/docs/ClangCommandLineReference.rst:3801 Pass -z to the linker - extraneous change? Comment at: clang/include/cla

[PATCH] D98466: [WebAssembly] Remove experimental SIMD instructions

dschuff accepted this revision. dschuff added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:191 -TARGET_BUILTIN(__builtin_wasm_qfma_f32x4, "V4fV4fV4fV4f", "nc", "simd128") -TARGET_BUILTIN(__builti

[PATCH] D131217: [clang][WebAssembly] Pass `-Wl,-no-type-check` through to the MC layer

dschuff accepted this revision. dschuff added a comment. This revision is now accepted and ready to land. This looks good to me from the wasm perspective Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131217/new/ https://reviews.llvm.org/D131217 __

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

dschuff added a comment. This broke us in emscripten as well (building with trunk clang against a recent-but-not-trunk version of libcxx). I can test the fix if you want. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116203/new/ https://reviews.ll

[PATCH] D78441: Delete NaCl support

dschuff added a comment. Sorry, no :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78441/new/ https://reviews.llvm.org/D78441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D78441: Delete NaCl support

dschuff added a comment. Deprecation is progressing (https://groups.google.com/a/chromium.org/g/chromium-extensions/c/v8H1UHnPotY/m/NmzrIv_VBAAJ) but we are still supporting it on some platforms, (and using clang's upstream support), so we aren't there yet. Repository: rG LLVM Github Monore

[PATCH] D78441: Delete NaCl support

dschuff added a comment. In D78441#4602312 , @brad wrote: > In D78441#4593287 , @dschuff wrote: > >> Deprecation is progressing >> (https://groups.google.com/a/chromium.org/g/chromium-extensions/c/v8H1UHnPotY/m/Nmz

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG220fe00a7c0f: [WebAssembly] Support `annotate` clang attributes for marking functions. (authored by brendandahl, committed by dschuff). Repository:

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

dschuff added a comment. Suggested edit to the commit description: "use musl and stddef.h at the same time" -> "use musl and clang's stddef.h at the same time" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159383/new/ https://reviews.llvm.org/D159

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

dschuff added subscribers: sbc100, sunfish. dschuff added a comment. The 2 common WebAssembly toolchain variants (Emscripten and wasi-sdk) use libcs that are derived from musl (a subset along with additions specific to their environments); they share the system-include configuration in `WebAsse

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

dschuff added a comment. The 2 common WebAssembly toolchain variants (Emscripten and wasi-sdk) use libcs that are derived from musl (a subset along with additions specific to their environments); they share the system-include configuration in `WebAssembly::AddClangSystemIncludeArgs()` in `clan

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

dschuff added a comment. Although having said all that, I guess a question for @aaron.ballman or other clang header experts: It does seem that many std C headers in clang are designed to handle this kind of case using `include_next` (e.g. stdint.h and stdatomic.h) but not all of them (e.g. stdd

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

dschuff added inline comments. Herald added a subscriber: ormris. Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96 + if (Arg *A = Args.getLastArg(options::OPT_O_Group)) { +if (const char *WasmOptPath = getenv("WASM_OPT")) { + StringRef OOpt = "s";

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

dschuff added a comment. If the SDK is distributed with the compiler and version-locked, it seems like it should be ok. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70500/new/ https://reviews.llvm.org/D70500 ___

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

dschuff added inline comments. Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96 + if (Arg *A = Args.getLastArg(options::OPT_O_Group)) { +if (const char *WasmOptPath = getenv("WASM_OPT")) { + StringRef OOpt = "s"; sunfish wrote: > dschuff wrote

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

dschuff added inline comments. Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96 + if (Arg *A = Args.getLastArg(options::OPT_O_Group)) { +if (const char *WasmOptPath = getenv("WASM_OPT")) { + StringRef OOpt = "s"; sunfish wrote: > dschuff wrote

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

dschuff added inline comments. Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96 + if (Arg *A = Args.getLastArg(options::OPT_O_Group)) { +if (const char *WasmOptPath = getenv("WASM_OPT")) { + StringRef OOpt = "s"; sunfish wrote: > dschuff wrote

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

dschuff added a comment. LGTM on the approach, just one more question on the wasm-opt flags. Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:105 +OOpt = "0"; + else if (A->getOption().matches(options::OPT_O)) +OOpt = A->getValue();

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

dschuff accepted this revision. dschuff added a comment. This revision is now accepted and ready to land. WRT the LTO directory name, there's theoretically the danger that someone (e.g. emscripten) could be doing a rolling release of the compiler and get invalidated within a major revision. But

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

dschuff added a comment. Also if at some point we are able to remove a bunch of the driver logic from emcc and move it here, (e.g. running clang to link instead of lld directly) we'll need to watch out for this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

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

dschuff added inline comments. Comment at: llvm/test/MC/WebAssembly/import-module.s:13 + .import_module foo, bar + .import_name foo, qux + What should happen if there's a directive that refers to a symbol that doesn't exist in the asm file? Repository: r

  1   2   >