In D56587#1407634, @dblaikie wrote:

In D56587#1393750, @dblaikie wrote:

In D56587#1393746, @probinson wrote:

In D56587#1393556, @dblaikie wrote:

I don't think anyone mentioned it, that's why I was bringing it up - there's always an option to not render any location if it's not possible/worth the work. That's all I was asking - is it worth the complexity? (I wasn't sure anyone needed it - but sounds like Sony does, reckon it's worth the tradeoff in complexity in LLVM compared to the work required to support this in the Sony debugger?)

NVPTX also would need it, because they are stuck on DWARF v2.

Any ideas if NVPTX hit this case? my understanding was that NVPTX has a fairly restrictive set of code or actions that can be used.

Ping on this - still wondering if anyone needs the complicated code or if we could get away with the GNU extension + DWARFv5 standard

LG

LG

LG

Ping!

In D58074#1399881, @lildmh wrote:

Thanks for the catch! I also change the name from OMPMappableExprListLocTy to OMPVarListLocTy to be more accurate. We can potentially factorize other varlist clause code with this. But it will be too large for this patch.

In D58074#1398187, @lildmh wrote:

Hi Alexey,

Again thanks for your review! The codegen completely ignores any mapper related info for now, so it should not crash map clause codegen. It also passed the regression test, so map clause codegen should be fine.

Check, please, that adding this to the map clause does not crash the codegen. Would be good to ignore this construct in codegen for now, if it is used in the code.

In D56587#1396249, @markus wrote:

NVPTX supports only DWARF2 and does not know anything about DWARF5 operations. Also, it does not support any type of the expression in the DWARF sections, except for <section_name>+-<int_offset>.

Isn't the latter a rather restrictive limitation that should be addressed in the NVPTX assembler?

In D56587#1396164, @probinson wrote:

+ @ABataev re the question whether NVPTX runs into the situation described in this review.

The Sony debugger guys are okay with using the GCC operator in a pre-v5 expression. So, tentatively, for all debugger tunings, we can emit that instead of the more complicated expression. That way we are emitting compliant expressions, and the info doesn't just disappear sometimes (a much worse outcome IMO). The only remaining question is my hypothetical about NVPTX.

Re branch operators, I thought Adrian was throwing that out there as a general concern; yes branch operators exist, and yes we don't use them currently. As David says, the assembler knows how to convert a label difference into a ULEB and it will all Just Work. If/when we ever need it to.

In D58074#1394993, @lildmh wrote:

Hi Alexey,

Thanks very much for your quick review!

For the codegen, currently I'm thinking to do it within declare target codegen functions. So there is not a very clear interface to do mapper codegen (also, there is not a clear interface to do map clause codegen now), and that's why I didn't have a stub left in this patch. Please see other detailed responses inline.

Also, need some kind of the stubbed codegen for the mapper

LG

In D57984#1394167, @rsmith wrote:

In D57984#1394067, @ABataev wrote:

In D57984#1394050, @rsmith wrote:

@ABataev Is it intentional that we do not propagate Allowed through labels? For example:

void f() {
  #pragma omp barrier // ok

label:
  #pragma omp barrier // error, "cannot be an immediate substatement"

label:
  ;
  #pragma omp barrier // ok
}

?

No, it is a bug.

Great, then I'll unify this new flag with the Allowed mechanism and fix the bug as part of this change. Thanks!

In D57984#1394050, @rsmith wrote:

@ABataev Is it intentional that we do not propagate Allowed through labels? For example:

void f() {
  #pragma omp barrier // ok

label:
  #pragma omp barrier // error, "cannot be an immediate substatement"

label:
  ;
  #pragma omp barrier // ok
}

?

In D57768#1392975, @bader wrote:

In D57768#1386941, @ABataev wrote:

In D57768#1386933, @bader wrote:

In D57768#1386924, @ABataev wrote:

This definitely requires a test.

@ABataev, I tried to find some tests on similar -fcuda-is-device and -fopenmp-is-device options, but I wasn't able to find a dedicated test. Could you suggest some examples testing similar functionality, please?

There are several similar tests:
OpenMP/driver.c, Driver/openmp-offload.c, Driver/openmp-offload-gpu.c. There is no absolutely the same test for OpenMP, since OpenMP has mo similar req for the offloading.

@ABataev thanks for the pointers. The uploaded patch adds two options:

• fsycl-is-device (front-end option)
• sycl-device-only (driver option)
  
  The driver tests you mention validate driver logic enabled by new options, which is not part of this test and I was going to add it later. I can split the patch and remove new driver option and leave only front-end option. Another option is to add driver logic that invokes the front-end compiler in "device only" mode. Which option do you prefer?

In D57768#1386933, @bader wrote:

In D57768#1386924, @ABataev wrote:

This definitely requires a test.

@ABataev, I tried to find some tests on similar -fcuda-is-device and -fopenmp-is-device options, but I wasn't able to find a dedicated test. Could you suggest some examples testing similar functionality, please?

This definitely requires a test.

Adrian, thanks a lot for the review!

Reworked according to the comments from Adrian Prantl.

Ping!

Ping!

LG