Looks good to me, but please give it a few days in case someone else has a comment.

LGTM

Formatting

I would have prefered a comment instead of a straight revert without giving me a chance to fix it.
Anyway, here’s a version that embeds bitcode instead of textual IR.

The debian pre-checkin test is unfortunately quite unstable. I see the same failures in D141469 for example.
This looks good to go.

Assemble callbacks in a struct, as suggested.

FYI, the CMake file should use PROJECT_SOURCE_DIR instead of CMAKE_SOURCE_DIR, otherwise it breaks builds that use CMake’s add_subdirectory. I put up D141521 to fix that.

Thanks for fixing the whitespace as well!

That looks good to me. I guess the same change applies to the duplicated code in flang, polly, lld and clang.

Rebased. Any concerns with the current patch?

The changes look good from my side. Can you add a unittest that converts an invalid DataLayout string to something legal?

For completeness, the patch avoids the creation of mentioned dir under some but not all circumstances?

I still get the ${CONFIGURATION} directory even with this patch

It is surprising to me that std::any can work without RTTI. Never thought it could be implemented.

ping for review

I believe the motivation here is the default behavior

I found a different way to fix the problem with msvc.
Marking the TypeId as non-const fixes the problem, at least in godbolt.

In D139006#3988215, @mkazantsev wrote:

So now every single test needs to be regenerated? It'll create straw diff from nowhere...

I guess this is fine to merge. I’ll leave it for a day in case someone has more comments.

Seems like we can’t… msvc fails to link
https://developercommunity.visualstudio.com/t/stdany-doesnt-link-when-exceptions-are-disabled/376072

I put up D139532, which includes the work done here and finishes it (also replaces any_cast with std::any_cast, compiles and passes llvm and clang tests).

Thanks for the review!
I updated the update_cc_tests tests and added a test where the FileCheck failed previously.

In D137866#3924203, @jsilvanus wrote:

Not your code, but:

Maybe it would be cleaner to check whether we need a frame object at all instead of adding special case handling for allocas?
Then, based on that condition, we could skip setup of the frame type and pointer, and leave the alloca's unchanged.

If that's too much of a change, I'm fine with the proposed change, as it just fixes the existing condition.

Looks good to me

LGTM

I’m not sure if it’s the case for all places (as CMAKE_CFG_INTDIR is not defined at install time), but I think the CMAKE_BINARY_LIBDIR introduced here serves the same purpose as the already existing LLVM_LIBRARY_DIR.
Same for CMAKE_BINARY_INCLUDEDIR, which is LLVM_INCLUDE_DIR and CMAKE_BINARY_BINDIR which is LLVM_TOOLS_BINARY_DIR.

This is probably caused by using CMAKE_CFG_INTDIR (indirectly) in more places. Seems like it expands to $(Configuration) for Visual Studio.
Searching more about it, it’s only set at build time, but not at install time, which is a problem as well.
On top of all, CMAKE_CFG_INTDIR is deprecated and superseded by generator expression: https://cmake.org/cmake/help/latest/variable/CMAKE_CFG_INTDIR.html

The build afterwards succeeded again. Seems like every ~20th build on that windows machine fails.

Thanks, looks good to me.

Looks ok to me in general.

In D130586#3731021, @Ericson2314 wrote:

Anyone have any idea what this Debian test failure is about?

Looks good to me, thanks!

I pushed a potential fix (removing the backslash) in 50716ba2b337afe46ac256cc91673dc27356a776.
I don’t know which buildbots to look at though, it looks like the OpenMP ones all succeed.

Looks good to me, I left three questions inline.

Looks good to me

I found a regression when llvm is added with CMake’s add_subdirectory. D130555 has a fix.