ping @lhames

I don't know if simply removing the tests is the right course of action, but yes I'd really love to get a clean check-all with Python 3.6

In D112759#3299743, @noajshu wrote:

In D112759#3271718, @Hahnfeld wrote:

The test still doesn't work for me on Python 3.6 because ThreadingHTTPServer was apparently introduced with Python 3.7.

Sorry again about this!
This will be fixed by D119096.

Any updates on this, do you plan to eventually land this change? FWIW this solves one third of https://github.com/llvm/llvm-project/issues/49036

The test still doesn't work for me on Python 3.6 because ThreadingHTTPServer was apparently introduced with Python 3.7.

In D113249#3117541, @carlosgalvezp wrote:

@Hahnfeld Are you satisfied with the replies to your questions? If so I can go ahead and merge.

Experimental support for __int128 is new in CUDA 11.5, not sure if Clang enables this for CUDA. The release notes also specify

builtin_assume can now be used to specify address space to allow for efficient loads and stores.

I'm not sure if it's actually correct to advertise full support for CUDA 11.5, but I didn't look into exact changes since 11.4

Sorry for the delay, looks good to me!

In D107049#2984835, @v.g.vassilev wrote:

It looks like we hit https://bugs.llvm.org/show_bug.cgi?id=49692

In D108404#2957095, @JonChesterfield wrote:

Reading the ninja file to determine what cmake's dependency graph looked like is a great idea. Going to use that in future, thanks for the comment!

I didn't follow the recent CUDA support in Clang very closely, but this makes sense to me.

(might be good to have an entry in the release notes?)

Otherwise LGTM

LG, should land after the other revisions are in.

LGTM

In D77670#2944192, @tra wrote:

In D77670#2943753, @Hahnfeld wrote:

@tra The split between LATEST and LATEST_SUPPORTED leads to very weird warning and error messages:

Agreed, it's far from ideal. There's also more than one issue involved.

@tra The split between LATEST and LATEST_SUPPORTED leads to very weird warning and error messages:

clang-14: warning: unknown CUDA version: cuda.h: CUDA_VERSION=11040.; assuming the latest supported version 10.1 [-Wunknown-cuda-version]
clang-14: error: cannot find libdevice for sm_20; provide path to different CUDA installation via '--cuda-path', or pass '-nocudalib' to build without linking with libdevice                                                                                                                    
clang-14: error: GPU arch sm_20 is supported by CUDA versions between 7.0 and 8.0 (inclusive), but installation at /usr/local/cuda-11.4 is 11.2; use '--cuda-path' to specify a different CUDA install, pass a different GPU arch with '--cuda-gpu-arch', or pass '--no-cuda-version-check'

LG

LG

LGTM, thanks

I think at least this patch needs a clang-format, maybe also applies to the other recent patches.

In D96033#2767884, @teemperor wrote:

In D96033#2766502, @phosek wrote:

In D96033#2766372, @v.g.vassilev wrote:

In D96033#2766332, @phosek wrote:

We've started seeing LLVM ERROR: out of memory on our 2-stage LTO Linux builders after this change landed. It looks like linking clang-repl always fails on our bot, but I've also seen OOM when linking ClangCodeGenTests and FrontendTests. Do you have any idea why this could be happening? We'd appreciate any help since our bots have been broken for several days now.

Ouch. Are the bot logs public? If not maybe a stacktrace could be useful. clang-repl combines a lot of libraries across llvm and clang that usually are compiled separately. For instance we put in memory most of the clang frontend, the backend and the JIT. Could it be we are hitting some real limit?

Yes, they are, see https://luci-milo.appspot.com/p/fuchsia/builders/prod/clang-linux-x64, but there isn't much information in there unfortunately. It's possible that we're hitting some limit, but these bots use 32-core instances with 128GB RAM which I'd hope is enough even for the LTO build.

I think the specs are fine for just building with LTO, but I am not sure if that's enough to for the worst case when running ninja -j320 with an LTO build (which is what your job is doing). Can you try limiting your link jobs to something like 16 or 32 (e.g., -DLLVM_PARALLEL_LINK_JOBS=32)

Can we get this fixed somehow? It's annoying that there is a test failure in Clang without building the OpenMP runtime, just because I have CUDA installed on my machine...

friendly ping :)

Superseded by D87413, I think.

Long addressed by D72779, but I can't seem to be able to close this (?)

In D95915#2541132, @tbaeder wrote:

That's what I was looking at right now as well, since using std::call_once() already means the methods can't be const anymore anyway. Might as well just cache the value.

My proposal would be to cache the return value of the three routines in ToolChain. This has the advantage that the values get parsed only once and there is at most one warning. I don't know how this plays with parallelization efforts, but I don't think we should worry about this right now, given the current code.

LGTM, thanks for the changes!

In D93169#2485228, @tianshilei1992 wrote:

In D93169#2484725, @Hahnfeld wrote:

Why is it necessary to write and compile a C program just to parse /proc/cpuinfo? Can this be done directly from CMake?

Unfortunately we can't. CMAKE_SYSTEM_PROCESSOR just reports aarch64.

Why is it necessary to write and compile a C program just to parse /proc/cpuinfo? Can this be done directly from CMake?

Add comment for find_first_existing_vc_file, thanks @scott.linder.

Ping

In D89974#2348176, @tra wrote:

CUDA path is sort of a global configuration parameter for all CUDA compilations. Perhaps we should consider allowing the user to specify a CUDA search path candidate via environment variable. This should allow transparently overriding preferred CUDA path without having to adjust all builds. I can't say I like it, but it seems to be the least bad way (that I can think of ATM) to address the dependency on something that only the end user would know for sure.

In D89974#2348124, @jdoerfert wrote:

I thought, right now we would configure clang with a cuda path XYZ

In D89974#2347979, @Hahnfeld wrote:

In D89974#2347938, @tra wrote:

I think the default should still let clang search for CUDA or require the user to provide correct CUDA path. "Use CUDA path discovered by CMake at build time" should be a non-default configuration option if/when it's needed and appropriate.

I agree here. It's definitely surprising to make it the *first* path because module loading another CUDA version and putting it into PATH is not recognized anymore.

In D89974#2347938, @tra wrote:

One concern I have is that the path we configure during clang's build is not necessarily the right choice for the user of clang we build. It's likely that the clang in the end will be used on a completely different machine.
E.g. official clang builds can not ever provide the same CUDA path for *all* users who end up using them. Requiring the rest to use a special option to make clang work again looks like an overall usability regression to me.

By the way, there's no commit list subscribed which sounds bad. Adding llvm-commits to give people a chance to be aware of this.

In D89915#2346071, @ldionne wrote:

@Hahnfeld Can you advise on the proper way to fix this? From git blame, it seems like you might know.

In D87084#2254700, @jdoerfert wrote:

Other places need to be updated too: data_sharing_init_stack_common for one.

Do we actually use this array? What happens if you make it a single element and update the function above?

In D83268#2135930, @JonChesterfield wrote:

Aside from the API stability concern this looks uncontentious. Removes dead arguments, generally makes things simpler. Thus LGTM.

@Hahnfeld @ABataev - are you sufficiently persuaded that preserving the current interface is not worth the development cost?

This is definitely not NFC and breaks API compatibility (but apparently nobody cares anymore?).

LGTM

The summary also talks about tools/multiplex/tests/*, is that information obsolete?

LGTM

LGTM

In D82963#2128507, @saiislam wrote:

Thanks @Hahnfeld. I realized that LLVM_MAJOR_VERSION was neither getting set in OpenMPTesting.cmake nor was it
inheriting it from anywhere else. So, OPENMP_TEST_COMPILER_VERSION_MAJOR was also getting set as empty, which
was getting propagated to lit by config.test_compiler_features. That is why "clang-11" was not getting recognized
as a valid target by lit-unsupported (though clang-11.0.0 would have worked). This change should fix this issue.

In D82718#2120414, @ye-luo wrote:

In D82718#2120235, @Hahnfeld wrote:

(In general, all patches must be sent to the respective -commits list. This also makes feedback more likely.)

Sorry, I was not aware of -commits list. What is it for exclusively? If there is policy or instruction , please point me.
I added OpenMP project tag. I you watch OpenMP project on differential. I expect you get notifications. Is it not the case?

(In general, all patches must be sent to the respective -commits list. This also makes feedback more likely.)

In D76012#2100421, @sylvestre.ledru wrote:

@protze.joachim @jdoerfert ompt-multiplex.h is installed by default in /usr/include/
others opemp headers are installed in /usr/include/openmp/

LIBOMP_HEADERS_INSTALL_PATH is the internal compiler where for example omp.h. I think the header tool is of relevance outside of Clang, so IMO it should go the general include/ directory.

I don't feel comfortable reviewing this change, maybe @phosek can take a look.

Does -fopenmp-version work with the Intel Compiler? How will this work if the GCC eventually gets support for OpenMP 5.0?

LGTM as well

In D78566#1995119, @jdenny wrote:

I just noticed another issue: ompt tests are skipped if FileCheck is not available. This is configured in openmp/runtime/test/lit.cfg. However, other openmp test suites that use FileCheck don't bother. Why is ompt different?

This patch makes not visible in all of the openmp project's test
suites. In all but libomptarget/test, it should be possible for a
test author to insert not before a use of the lit substitution for
running a test program.