@rnk - okay, that seems reasonable - can you please update the documentation on how to setup the Visual Studio Developer command prompt such that you can use the tools (assuming that Git is setup by the Visual Studio Installer) then?

Probably just a replay of GDB protocol packets connected to a testing server would work. But, I don't know if there is infrastructure for that yet.

Previously we would have emitted Foo [[:space:]] . right?

Would be nice to change the filecheck prefixes to CHECK-... to separate the CHECK and the item being checked.

Actually, I'm not sure if that is a bug in libstackunwind, but rather in LLVM. I think that this is the behaviour that GCC has specifically for the eh_frame section. This constraint is not part of the DWARF spec, but eh_frame even though it is similar to debug_frame, is not part of the DWARF spec, it just uses a similar encoding. The change itself looks good, but, please wait for the discussion to complete before committing this.

Thanks!

@rnk @zturner @ruiu - can we come to a decision here?

SVN r354045

@sgraenitz - not really ... the find_package will load the LLVMConfig.cmake that is generated. We only have what we keep in there.

Add HINTS

@labath - absolutely, that I don't have a problem with. I think that having the additional LLDB specific paths with LLDB_PATH_TO_* is better done by using the standard CMake mechanisms.

Yes, both paths currently work since the LLDB_PATH_TO_* variables are just hints to where to look. It just seems unnecessary to have the custom variables when CMake has a mechanism for directing the build to look for packages in a certain location.

LG with the additional test suggestion

@zturner I think that moving to the integrated lit shell is a great idea, as it really does lower the barrier to entry and makes testing stuff on Windows much easier.

Which version of the patch did you use? L13 in the current version sets LLVM_MAIN_INCLUDE_DIR specifically for that case.

@zturner, to your point, I'd like to add the fact that it took a bit of effort to even figure out that the problem was so nicely tucked away in the corner of lit because the failure scenario is pretty absurd (-ENOENT). IMO, this makes this even more insidious because it will cause a lot of people to get confused or possibly give up. But, perhaps I just am holding on to tightly to my idea that things should just work and the testing infrastructure should encourage people to add tests rather than demotivate them.

@sgraenitz yeah, I passed LLVM_DIR and Clang_DIR, but, this is for a standalone build, so I think that it is pretty reasonable to ask that the user tell us where LLVM and Clang are built. Although, if you install LLVM and Clang to your root (like on Linux), you do not need to specify that because it will search the system by default.

@ruiu - to your point of "it being better in the future" I think is false hope. The backwards API compatibility will prevent Microsoft from changing the behaviour of the APIs without an explicit opt-in. Otherwise, applications need to be rewritten, which has long been the case, you can use the NT style paths all the way back to Windows XP I believe, but applications aren't being rebuilt and redistributed.

@ruiu it is only "removed" if you opt into it by modifying the registry on a per program basis. I don't think that is really a reasonable approach.

@kastiglione - bleh, seems that this got lost. This seems like a good fix that we should get in.

Switch to SHFileOperationW for the recursive removal. Refactor lit.util.mkdir_p to expose lit.utl.mkdir and update that to support long file paths. This makes lit more robust on Windows against creating long path temporary directories.

Okay, seems like the way to handle this is to switch to SHFileOperationW using ctypes. I believe that the win32py support that is needed for this should be part of the default python installation on windows (at least it is part of the Visual Studio python distribution).

Ugh, right ... I think that I had tested it against an empty directory! It was run early in the test. I'll flesh this out with @zturner's idea of extract it into a helper.

@ruiu, no unfortunately, not all the paths can be shortened in the swift test suite since it is such a heavy user of the clang modules, modules cache paths and module naming structure in clang is a problem

comments, error handling, context

That doesn't work with python 2.7 as that uses RemoveDirectoryA, which does not support it.

Fixes

Restore LLVM_MAIN_SRC_DIR cache variable

This doesn't break the build, we check that the path exists, and find_program will not error out. You can still specify LLVM_LIT_EXE to specify the lit.py to use. Is that not sufficient for your needs?

@mgorny, no, you should specify -DLLVM_DIR=/path/to/where/the/config/lives instead.

This actually breaks the compatibility with the existing tools. The quoted string is supposed to be interpreted as a *single* entry. I think that if you want to support something like this, it should be a separate flag for this behaviour.

Sure, the new license is at https://llvm.org/LICENSE.txt (Apache)

@sarveshtamba - just to make sure, you are aware that the license for LLVM has changed and that this work is acceptable to be published under the new license?

Do you need me to commit this on your behalf (as last time)?

double should be safe for ARM DWARF EH, though, technically, long double is more appropriate of a type definition (the FPU state that is saved should be the widest floating point type). That would be long double (aka FP80 on x86, FP128 on AArch64/PPC, FP64 elsewhere). This happens to work because ARM uses FP64 irrespective of DWARF or EHABI.

Did we not address the PPC related checks earlier? IIRC, the problem was that gcc does not define __PPC__ but does define __ppc__. The problem with std::call_once was not related to endian, so why not extend the condition on L32 to include __PPC__?

I personally prefer the use of target_link_libraries rather than the custom stuff added in the add_lldb_tool.

Clever!

Shell is not available on Windows, python however is considered a portable shell within the project. There are a number of tools within the LLVM project that require python, I'm not sure that removing the dependency is the right thing to do here.

I'm not sure how I feel about the additional parameters. What do you think about packaging up the filters into a structure and passing a single "blob" as the filter?

The only minor nit is the preference for Known in some of the operand checks. IIRC clang doesn't get 100% of the annotations, so even if something is not marked as noread, it may be noread. Yes, its nit-picky, but, I think it helps when dealing with code that you are unfamiliar with (or happen to page out the context from clang).

SVN r348970

I like the idea for the simplification, I'll do that as I commit it. Thanks for the hints @nemanjai!

@nemanjai - I'm not following you with regards to the libc++ being used. This is simply used to replace the use of std::call_once with a local implementation, it doesn't introduce a dependency on compiler-rt at all and it doesn't change the C++ runtime that you are depending on.

SVN r348939

It is okay to fallback to the hand-rolled CAS based once initializer. It would be nice to have a clearer explanation for why the C++ runtime on the target cannot support std::call_once which is a C++11 standard function.

Please do clang-format this. If this is already in the wild, then, I suppose that we don't have the option, but, this seems like something that should be written by the author of the assembly file. This is really the same as the emission of the emission of the directive for the GNU noexecstack, although, there is --noexecstack.

This really feels odd. Why not expect that the developer will add the content themselves? I'm not sure I understand the motivation for this change. I think that this should be easy to write a test case for as well.

Unfortunately, the TAPI releases are in the wild, so we need to support the old files. It would be nice to have some sort of marker on the file (similar to a shebang) to indicate that it is a TAPI file.