[Openmp-commits] [PATCH] D60578: [OPENMP][NVPTX]Fix dynamic scheduling in L2+ SPMD parallel regions.

[Hal Finkel via Phabricator via Openmp-commits]

hfinkel added a comment.

In D60578#1465425 <https://reviews.llvm.org/D60578#1465425>, @Hahnfeld wrote:

> In D60578#1465420 <https://reviews.llvm.org/D60578#1465420>, @hfinkel wrote:
>
> > In D60578#1465371 <https://reviews.llvm.org/D60578#1465371>, @Hahnfeld wrote:
> >
> > > I think we need to be careful about adding `nvptx` tests to `libomptarget/test`: They can be executed using Clang later than 6.0.0, but that version wasn't able to offload to GPUs. Given that the changes are limited to `libomptarget-nvptx` (because its parallelism is kind of special), I think the new test should go to `libomptarget/deviceRTLs/nvptx/test`. Just my 2 cents...
> >
> >
> > I don't see anything in this test that is nvptx specific. Is there something about the semantics that make it specific to nvptx? We need to build of a suite of tests for accelerator offloading in general. We'll have other accelerator backends (e.g., for AMD GPUs), and the offloading tests should apply to them too.
>
>
> First, I agree that the test is not specific to `nvptx` and should pass for all targets. However (at least so far) `libomptarget/test` is for tests that exercise the target-agnostic part of `libomptarget`; like starting `target` regions, environment variables, mapping etc.
>
> > Also, I don't understand what Clang 6 support has to do with adding tests... clearly, we'll add tests for bugs, or already have, that will then fail on older versions of Clang.
>
> In that case these tests need to be marked `UNSUPPORTED` for versions of Clang that will not pass them. There's infrastructure for that, but it's not applied in the current form of this patch.


Okay. This patch review is not the right place to discuss the libomptarget support for old Clang versions. We should have a separate thread on this subject.

> 
> 
>> And if I target is not supported, the associated libomptarget-compile-run is ignored, no?
> 
> Yes and no: Yes, if a plugin (meaning the library that interfaces with the vendor libraries for launching `target` regions) is not compiled, the `libomptarget-compile-run` for that target expands to echos. However, there is currently no way of finding out if a given test can actually run (for example there needs to be a GPU plugged into the system). In theory you could use vendor commands like `nvidia-smi` to query that, but that still does not guarantee that the tests have a chance to pass (because of various reasons; the one that I care most about is that we have our GPUs configured in Exclusive mode, so if there's a process already running on the GPU, all others that try to create a context will get a runtime error) and IMHO it would be a poor development if `check-openmp` would simply stop to work in these cases.
>  (Side note: CUDA in Clang does the same, they have tests in `test-suite` that can actually be run on real hardware; the Clang tests just check the generated code.)

What check command do you run to run these nvptx tests?

> I can see your point that having generic tests live below `libomptarget-nvptx` is not ideal, but I think it's the best place we have right now given that apparently nobody plans to work on more infrastructure (which is sad).

I'm aware of several groups working on different libomptarget plugins and other related things (including my team), so I suspect that reality might not be as sad as you believe. Nevertheless, what infrastructure do we actually want here? Should we have the ability to ask make check-openmp to take a list of targets to use to run all of the offload tests so that the user can specify the names of the targets that will actually work?

In any case, let's move forward with adding this test in that directory, and then we'll address the infrastructure issue as follow-up work.


Repository:
  rOMP OpenMP

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60578/new/

https://reviews.llvm.org/D60578