I expected more changes to be necessary, to make it work with litsupport modules, perf/timeit, statistics, compile time (Interestingly, CMAKE_Fortran_COMPILE_OBJECT is already defined), LNT, ... . But if we find they don't work yet, we can fix that later.

Consider clang-format before pushing to main.

LGTM

LGTM, thank you.

LGTM

Alternatively, we could rely on SimplifyCFG to remote such empty BBs?

I don't understand the motivation. Sharing RuntimeCheckingPtrGroup between multiple objects will increase coupling between them.

I would have done an unequal: if (NOT TARGET_OS STREQUAL "Darwin"). libIntl is part of the GNU user space environment, not the kernel, e.g. also apply to GNU/kFreeBSD.

• Address reviews

ping

LGTM

You are right, FindGettext seems to look for some commands, not the library.

Can I help fixing the Windows build problem?

Thanks. Still LGTM.

LGTM

Usually system introspection happens in the top-level CMakeLists.txt, but test-suite allows to selectively include subdirs, so it is a good location to do here.

• Avoid pre-merge check warning about using auto for an iterator.

• Extract changes not directly related to tileLoop into D93088
• Address pre-merge checks

Obsoleted by D91210

Thanks, LGTM.

LGTM

OMPBuilder part looks good to me, but probably an MLIR person should accept this patch.

LGTM

Sorry for the wait. Phabricator did not send me notifications for the all the updates. Anyone has an idea why?

Looking at CodeGenFunction::GenerateOpenMPCapturedVars, the condition if (!CurField->getType()->isAnyPointerType()) seems to match what is done here, but I wonder whether it is always necessary. Is the additional indirection necessary for pointer-sized data types (e.g. intptr_t, or smaller)?

So why not creating a OpenMPIRBuilder::createWorksharingLoop that can be used by clang as well?

The intent was to create a OpenMPIRBuilder::createWorksharingLoop that takes a CanonicalLoop as an argument to be workshared. The heavy lifting would be done by createWorksharingLoop such that its logic does not need to be duplicated by clang and the OpenMP dialect.

In D91641#2401724, @serge-sans-paille wrote:

In D91641#2401024, @Meinersbur wrote:

There seems to be one set of braces too much, makes me wonder how it ever worked.

The « extra » set of brace was for list initialization, using that rule (from https://en.cppreference.com/w/cpp/language/list_initialization)

Otherwise, the constructors of T are considered, in two phases: 

    All constructors that take std::initializer_list as the only argument, or as the first argument if the remaining arguments have default values, are examined, and matched by overload resolution against a single argument of type std::initializer_list 

   If the previous stage does not produce a match, all constructors of T participate in overload resolution against the set of arguments that consists of the elements of the braced-init-list, with the restriction that only non-narrowing conversions are allowed. If this stage produces an explicit constructor  as the best match for a copy-list-initialization, compilation fails (note, in simple copy-initialization, explicit constructors are not considered at all).

Thank you for you the fix!

There might be other cmake configuration parameters used for introspection, such as ZLIB_INCLUDE_DIR (if using zlib), <PackageName>_ROOT etc.

From the array element type, the compiler should know which type to brace-initialize. There seems to be one set of braces too much, makes me wonder how it ever worked.

In D91470#2395495, @mehdi_amini wrote:

In D91470#2395433, @mehdi_amini wrote:

We should dig in the OpenMP side as well.

What do you mean by that?

I mean that commenting out OMP.cpp here: https://github.com/llvm/llvm-project/blob/master/llvm/lib/Frontend/OpenMP/CMakeLists.txt#L6 and running ninja check-llvm just works.

Is this obsolete after D91470 has been committed?

Like IRBuilder and OpenMPIRBuilder, I think the OpenACC stuff is meant to be used by frontends outside of LLVM (in the sense of /llvm/*) and thus should be exposed as a component and llvm-config, but tested independently of those outside users. OpenACC is standardized for C/C++ as well.

This looks like an overdue fix. Thanks.

In D90848#2394970, @serge-sans-paille wrote:

There was an inconsistency in the LLVMBuild.txt for LLVM Frontend: OpenACC wasn't listed but it was part of the CMake infrastructure. A side effect of the llvmbuildectomy has been to remove that inconsistency and register OpenACC as a regular component.

LGTM

In D89158#2389520, @aeubanks wrote:

Thanks, I wasn't aware of this. Looks like other opt -passes='default<O#>' tests also look at if it present or not.

Could you try configuring LLVM with -DLLVM_BYE_LINK_INTO_TOOLS=ON? I assume the test will break because it unconditionally adds a FunctionPass.

This looks like a major simplification of the build system. Code looks good, I confirmed it is working for my Windows build, Polly (i.e. statically linked pass plugins) is listed with --libs.

Thanks, looks good to me.

My point is that it is possible for a pass plugin (such as llvm/examples/Bye) that can add themselves to the -O0 pipeline. If those plugins are linked statically, it will break tests that assume that no passes are added at those extension points.

In D89158#2384156, @aeubanks wrote:

Are there passes that should be running at -O0 that aren't target specific? Otherwise the callbacks should only add passes if the optimization level isn't -O0.

Last update pushed only the change

Missed reference of CreateXXXX in comments

Thanks for your effort. I love it. Seems to remove complexities of the buildsystem and the ugly split of having to define library dependencies in two files.

Did you consider just removing the -NEXT suffix? Polly might not be the only case, other statically linked pass-plugins may also insert themselves at extension points.