- User Since
- Jan 7 2013, 9:35 AM (288 w, 3 d)
Thu, Jul 12
I think I actually prefer the approach of linking this code in from a library rather than generating N copies of it. It's more consistent with how system code is usually included into programs (i.e. linking it in rather than generating it magically from the compiler). It's also easier to read and understand for non-compiler developers.
@jfb just as a sanity check, it should always be valid to just "upgrade" LLVM's orderings and select them all as the sequentially consistent wasm ops, right?
Mon, Jul 9
There should also be a few tests that orderings other than seq_cst get selected.
Tue, Jul 3
This change is really quite nice and minimal other than the renaming.
Mon, Jul 2
Fri, Jun 22
Thu, Jun 21
Wed, Jun 20
A nit about the commit message: it's not just LTO passes that make assumptions about known library functions, lots of passes do. Also I might be just a little bit more explicit than "when statically linking libc" and make it "when including libc in LTO"
Jun 19 2018
Looks pretty reasonable otherwise. I like the MIR unittest 👌
Jun 18 2018
LGTM with the tweak to the comment.
Since it fixes the build and is a trivial change, I think you should just go ahead and commit this.
LGTM; I think this model definitely makes more sense in CFG-based IR.
Jun 14 2018
This does seem a little odd, but I think it makes sense given the special nature of functions in the wasm object format.
Jun 4 2018
LGTM, we can remove any code relating to binary output with the ELF triple now. Hopefully not much longer before we can remove the ELF triple entirely.
No worries; the WebAssembly target is still an "experimental" target and doesn't build by default (even you build "all" targets) unless you also add -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD. So this happens from time to time.
May 31 2018
Yes, still LGTM
May 29 2018
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 that chain, or does it matter?
May 17 2018
The Wasm code looks good with the one comment so LGTM if @majnemer is OK with the modification to WinEHPrepare.
Thanks for your patience on this, I really do think this is a big readability improvement.
May 16 2018
May 15 2018
Also if you use the LINK_LLVM_DYLIB CMake mode (as our bots do) you should test the regular non-dylib static link build.
May 10 2018
May 7 2018
haven't looked at most of the code, but thought about the register operand issue.
Apr 6 2018
Apr 5 2018
Apr 4 2018
LGTM for the encoding... I hope the -1 offset wasn't being generated by the compiler though?
Apr 2 2018
Mar 30 2018
Mar 29 2018
Mar 28 2018
Otherwise it looks good to me, although @majnemer would know more about the subtleties of what IR actually gets generated.
Mar 27 2018
Mar 21 2018
Sorry about that. Although I'm confused though why setting 'ShouldEmitMatchRegisterName' (which I added right before commit when I saw this warning myself) (https://github.com/llvm-mirror/llvm/blob/master/lib/Target/WebAssembly/WebAssembly.td#L84) didn't work.
Mar 20 2018
- add braces in else
Mar 19 2018
Mar 15 2018
I think i'd be in favor of letting this patch go in as it is for now, since it uses our existing syntax, and then have a separate discussion of how to change it on tool-conventions.
Do you want me to commit this?
Mar 14 2018
I think the symbol table (and any names that actually have meaning to the compiler and/or linker) should be mangled names, and only the name section should have demangled names. I also think it makes sense for libObject to only use the symbol table and ignore the name section. In other words, the name section should be thought of as metadata or debug info and in that sense shouldn't be "trusted" to always be correct (although of course we should preserve it wherever we can).
Mar 13 2018
LGTM, and confirmed that it makes libcxx configure successfully.