- User Since
- Dec 9 2012, 11:41 PM (236 w, 6 d)
Address feedback. Also fix the case that was previously not working. This now covers all the various cases that have been discussed.
Fri, Jun 23
Ah, thanks for the explanation @efriedma.
This is a step in the right direction. Although the NSDMI cases and default parameter value cases are not yet handled, they break due to tracking of the global mangling number tracking, not due to the scheme.
@efriedma I think that Im still not understanding the case that you are trying to point out. How do we end up in a state where the block invocation function is referenced external to the TU? The block would be referenced to by name of the block, no? AFAICT, this is for local storage in a block, which is scoped to the block invocation function, which is TU local, and will be referenced by the block_literal (which contains the block invocation function and is dispatched via the BlocksRuntime).
@efriedma which bit of the Itanium mangling should I be looking at? A BlockDecl does not have visibility associated with them, so Im not sure what I should be checking to see if the block is visible or not. What is the best way forward for finishing this up?
Add additional test cases, improve coverage and mangling.
Thu, Jun 22
@efriedma hmm...using getBlockManglingNumber causes the name to be duplicated. Ill look into that. However, wouldn't all the block invocation functions be defined and COMDAT'ed?
Tue, Jun 20
Sat, Jun 17
Thu, Jun 15
Sun, Jun 11
Fri, Jun 9
Thu, Jun 8
Mon, Jun 5
Sat, Jun 3
Thu, Jun 1
@ismail the current version of the patch should have those requested changes :-).
Wed, May 31
Im not sure that this is a reasonable patch. I think that specializing a AddressSpace for _WIN32 is more likely what we want. Furthermore, this actually should be more restrictive, since the unwind library cannot be used for Windows -- the unwinding model on Windows is completely different. This only makes sense for MinGW and cygwin, and so we should ensure that the environment matches that.
Sun, May 28
Thanks for looking into this, its been on my list for a while now.
May 24 2017
Please clang-format before committing this.
May 23 2017
Looks generally pretty good. This is going to be a pretty cool addition!
Modulo the IT block macros, LGTM.
May 22 2017
May 21 2017
Can you please ensure that you cross-port this into llvm/lib/Demangle/ItaniumDemangle.cpp?
May 16 2017
May 15 2017
May 14 2017
Sure, a _LIBCPP_MSVCRT_LIKE WFM. I just want to make sure that we don''t conflate the underlying libc implementation with the Win32 API set.
I think that we should sink the min/max checks into __undef_macros. I don't like the idea of littering that check everywhere.
May 9 2017
May 6 2017
May 5 2017
May 4 2017
May 3 2017
May 1 2017
@mstorsjo Yeah, clang supports proper TLS on Windows on ARM. This is only needed if you are explicitly going out of your way to use the emulated TLS (via -femulated-tls). Otherwise, this code path shouldn't be hit.
All of this functionality is in the current tool AFAIK. Furthermore, this has been out of date for so long that it's not particularly useful anymore. Commandeering to close the differential.
Apr 30 2017
Doing this on just the MinGW build seems reasonable. Most of the library shouldn't be needed for Windows.
Apr 28 2017
Apr 27 2017
@majnemer thats the idea :-). Passing either triple to clang would give the right behaviour (clang will normalize the triple before passing it to cc1).
Yes, the idea is that the textual IR uses the correct triple, unless it is overidden at the command line.
Apr 24 2017
Why not always replace the extension? Windows doesnt require the .exe suffix IIRC.
Is this to actually get the correct GCC search dir? Your test doesnt really test anything AFAICT, as it is just invoking clang with a target that it would accept anyways.
I think that you should mutate the environment in the canonicalisation phase of the triple. That will allow you to use armv7-suse-linux-gnueabi and armv7-suse-linux-gnueabihf in the frontend, but have the backend always get armv7-suse-linux-gnueabihf.
Apr 21 2017
Apr 20 2017
Apr 19 2017
This would ideally wait for the change that @jbcoe has in the works to enable python 3, but the change itself is fine.
Thanks for doing this cleanup!
I think it would've been nice to split this up into the changes for map/filter rather than group it together. But sure, this looks reasonable.
Please add a test case. The change itself looks reasonable.
Apr 18 2017
This is opt-out by default, so it shouldn't have any impact on users by default. When building for tooling, this should allow smaller builds for those.
Apr 14 2017
Apr 13 2017
Minor comments on the test, but LGTM otherwise.