In D72741#1827150, @ftynse wrote:

@tra, your rule adds you as blocking on all of MLIR :)

Tank you for splitting the patch. For CMake parts @beanz would be the person to make the call.

Oops. Herald rule misfired.

In D72806#1825400, @DieGoldeneEnte wrote:

In D72806#1825333, @DieGoldeneEnte wrote:

In D72806#1824560, @yaxunl wrote:

What's the use case of this change?

Normally clang needs to call opt/llc/lld from the same directory of clang. Why do we need to find them in other directories?

My motivation is the nix-package manager, which has llvm, lld and clang in different packages (which results in different directories).

LGTM. Added @beanz for the review of CMake file changes.

What's the use case for this flag?

In D69990#1790154, @6yearold wrote:

It's somewhat old and misses few glue functions needed by CUDA-10, but it should work well enough for CUDA-9.

Interesting, thanks for sharing. However, at quick look, it seems to require other CUDA libraries (libcuda, libcublas, etc.), which also aren't available for FreeBSD.

... I'm curious if it's particularly useful. Last time I checked NVIDIA didn't ship libcudart for FreeBSD and without it it's rather cumbersome to use CUDA in practice.

I wonder if this patch will help with this case:

Looks good to me overall. I've pinged rsmith@ to double-check that we're covering all possibilities for non-local variable init.

+1 for tests.

LGTM, though I'm curious if it's particularly useful. Last time I checked NVIDIA didn't ship libcudart for FreeBSD and without it it's rather cumbersome to use CUDA in practice.
You can compile a kernel, but kernel loading, launching, and related data transfers will all need to be done via driver API. It should be possible to implement a functional replacement, but I'm not aware of any existing open-source implementations. I'm also not sure if clang will be able to deal with CUDA headers correctly on FreeBSD as CUDA headers do sometimes seem to rely on implementation specifics of Linux headers.

Calling @rnk for Windows know-how.

Apologies for the delay with my response.

Thank you!

LGTM in general. Nit about test name and test scope.

@arsenm has a point. We can do it in clang, and it seems to be a better long-term solution compared to patching-up the inputs' AS that we've done for NVPTX and, now, AMDGPU.

Could you, please, give us a bit more context and provide more info for the questions @rjmccall and I asked before?

Would I be too far off the mark to summarize the situation this way?

@echristo Eric, any thoughts/concerns on the overall direction for the driver?

In D60220#1725633, @dim wrote:

Hm, I would really say that __isnan and the other __ prefixed functions are Linuxisms, or more accurately, glibc-isms. They also don't exist on e.g. macOS:

Why can't the regular isnan be used instead? Or is this a CUDA-specific requirement? (Apologies, but I know next to nothing about CUDA :) )

@rnk Reid, would you be the right person to look at the change on the Windows' side?

The CUDA builtin library is apparently compiled in C++ mode, so the
assumption of convergent needs to be made in a typically non-SPMD
language.

In D60220#1723458, @emankov wrote:

In D60220#1723350, @6yearold wrote:

I'm seeing quite similar errors on FreeBSD with Clang 8 and 9:
Any idea how to fix this?

It looks like CUDA doesn't support double argument for device function __isnan on FreeBSD.

Perhaps we should rename -fcuda-allow-variadic-functions to -fgpu-allow-variadic-functions after this patch.

Added a comment

Thank you for adding the warning. One small nit about the name. LGTM otherwise.

Nice. I wish we could do that for CUDA.

Neat. I like the visual cues showing what gets passed on to the next processing stage.

This is... rather oddly-structured output. My brain refuses to accept that the most-indented phase is the input.
Perhaps we should do llvm::errs().indent(MaxIdent-Ident). This should give us something like this (withMaxIdent=9), which is somewhat easier to grok, IMO:

Could you give an example of before/after output?

@rsmith Richard, could you take a look, please? Lambdas, mangling, ODR rules & ABI scare me. :-)

Addressed Tim's comments.

LGTM.
Sometimes I wish it would be possible to specify some of the -verify diagnostics in temporal order, as opposed to tying them to locations. I.e. in this case it would be way more useful to see the call stack leading to the error at B::B. Oh, well.

In D68578#1698864, @t-tye wrote:

From a source language point of view, the device function comprises the code that is launched as a grid. We need this fact to be present in the symbols used. Only the device function should have a symbol name matching the mangled name of the device function.

Maybe, add a TODO to eventually remove .d .
.cui should probably remain as it's yet another variant of preprocessed output that we allow bundling for C/C++.

Added Manuel as someone familiar with tooling.

TL; DR;
+1 to formalizing how we want -M*/-E/-S/-emit-llvm/-fsyntax-only to behave.
OK with -M/-E/-S defaulting to host, and erroring out if applied to multiple sub-compilations.
I'm still convinced that the tooling issue with multiple subcompilations is orthogonal to this change and should be handled in libclang and that -fsyntax-only should not default to one sub-compilation.

In D68587#1698102, @hliao wrote:

for most compilation tools, single input and single output are expected. Without assuming -fsyntax-only alone is host-compilation only, that at least run syntax checking twice.

I'm fine with this for -E/-M,

In D68578#1697851, @yaxunl wrote:

In D68578#1697822, @tra wrote:

Could you elaborate on how exactly current implementation does not work?

I would expect the kernel and the stub to be two distinct entities, as far as debugger is concerned. It does have enough information to track each independently (different address, .stub suffix, perhaps knowledge whether it's device or host code). Without the details, it looks to me that this is something that can and should be dealt with in the debugger. I've asked the same question in D63335 but I don't think I've got a good answer.

HIP debugger is a branch of gdb and the changes to support HIP will be upstreamed. When users set break point on a kernel, they intend to set a break point on the real kernel, not the device stub function. The device stub function is only a compiler generated helper function to help launch the kernel. Therefore it should have a different name so that it does not interfere with the symbol resolution of the real kernel.

Could you elaborate on how exactly current implementation does not work?

LGTM. Thank you for fixing this.

Could you also add a check for -S w/o -emit-llvm, too ? AFAICT it currently wants to produce a bundle, which is not very helpful.

Does it produce textual IR if used with -S?

The functionality looks generic enough. Should it be just flink_builtin_bitcode?