clang: Remove unnecessary host-supports-cuda from test

[arsenm]

No description provided.

[arsenm]

• clang: Remove unnecessary host-supports-cuda from test #171174 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-clang

Author: Matt Arsenault (arsenm)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/171174.diff

1 Files Affected:

• (modified) clang/test/Headers/cuda_with_openmp.cu (+1-1)

diff --git a/clang/test/Headers/cuda_with_openmp.cu b/clang/test/Headers/cuda_with_openmp.cu
index 8ea0de5972ff2..efde4ecdc6626 100644
--- a/clang/test/Headers/cuda_with_openmp.cu
+++ b/clang/test/Headers/cuda_with_openmp.cu
@@ -2,7 +2,7 @@
 // Reported in https://bugs.llvm.org/show_bug.cgi?id=48014
 ///==========================================================================///
 
-// REQUIRES: nvptx-registered-target, host-supports-cuda
+// REQUIRES: nvptx-registered-target
 
 // RUN: %clang -x cuda -fopenmp -c %s -o - --cuda-path=%S/../Driver/Inputs/CUDA/usr/local/cuda -nocudalib -isystem %S/Inputs/include -isystem %S/../../lib/Headers -fsyntax-only
 

[Artem-B]

The change looks OK to me, but I don't have enough context to tell if there's something special about OpenMP parts.
@jhuber6 may be the best person to stamp this PR.

[arsenm]

This came from 6106b94 but is the wrong condition, I can't guess what zos actually cares about

[jhuber6]

Should work since we pass -nocudalib, probably just leftover so it's best to get rid of it. In the past I've tested stuff like this by just temporarily renaming my local CUDA installation so it wasn't found by default

[perry-ca]

This test will fail on z/OS without a resource requirement for cuda. I get these errors when running this test:

clang: warning: argument unused during compilation: '-c' [-Wunused-command-line-argument]
In file included from <built-in>:1:
In file included from ./../../lib/Headers/__clang_cuda_runtime_wrapper.h:158:
./../../lib/Headers/__clang_cuda_math.h:284:13: error: use of undeclared identifier 'INT_MAX'
  284 |   if (__b > INT_MAX)
      |             ^~~~~~~
./../../lib/Headers/__clang_cuda_math.h:286:13: error: use of undeclared identifier 'INT_MIN'
  286 |   if (__b < INT_MIN)
      |             ^~~~~~~
./../../lib/Headers/__clang_cuda_math.h:291:13: error: use of undeclared identifier 'INT_MAX'
  291 |   if (__b > INT_MAX)
      |             ^~~~~~~
./../../lib/Headers/__clang_cuda_math.h:293:13: error: use of undeclared identifier 'INT_MIN'
  293 |   if (__b < INT_MIN)
      |             ^~~~~~~
4 errors generated when compiling for sm_52.

Is there a better resource to require? This does require the the target support cuda.

[perry-ca]

I noticed the test case includes real system headers (see the option -isystem %S/../../lib/Headers) and not platform independent stubs. That makes the lit test highly unportable.

[arsenm]

This test will fail on z/OS without a resource requirement for cuda. I get these errors when running this test:

clang: warning: argument unused during compilation: '-c' [-Wunused-command-line-argument]
In file included from <built-in>:1:
In file included from ./../../lib/Headers/__clang_cuda_runtime_wrapper.h:158:
./../../lib/Headers/__clang_cuda_math.h:284:13: error: use of undeclared identifier 'INT_MAX'
  284 |   if (__b > INT_MAX)
      |             ^~~~~~~
./../../lib/Headers/__clang_cuda_math.h:286:13: error: use of undeclared identifier 'INT_MIN'
  286 |   if (__b < INT_MIN)
      |             ^~~~~~~
./../../lib/Headers/__clang_cuda_math.h:291:13: error: use of undeclared identifier 'INT_MAX'
  291 |   if (__b > INT_MAX)
      |             ^~~~~~~
./../../lib/Headers/__clang_cuda_math.h:293:13: error: use of undeclared identifier 'INT_MIN'
  293 |   if (__b < INT_MIN)
      |             ^~~~~~~
4 errors generated when compiling for sm_52.

Looks like a missing limits.h include in __clang_cuda_runtime_wrapper.h?

Is there a better resource to require?

This shouldn't require any host resource

This does require the the target support cuda.

No it doesn't, the extent that Cuda is required is the fake path to the headers

[perry-ca]

Another thing with this test is the -fopenmp option. z/OS doesn't support openmp. It at least needs a line saying unsupported when targeting z/OS. The problem with not using fake headers also needs to be addressed so testing other targets from z/OS works.

[arsenm]

I noticed the test case includes real system headers (see the option -isystem %S/../../lib/Headers) and not platform independent stubs. That makes the lit test highly unportable.

Those aren't real system headers, that's pointing to clang/test/Headers/Inputs/

[perry-ca]

I noticed the test case includes real system headers (see the option -isystem %S/../../lib/Headers) and not platform independent stubs. That makes the lit test highly unportable.

Those aren't real system headers, that's pointing to clang/test/Headers/Inputs/

It's actually clang/test/Headers/../../lib/Headers which becomes clang/lib/Headers. Those are the real clang wrappers headers and they include system headers that don't resolve to the fake ones. For some reason the -H option doesn't work with the cuda processing otherwise I'd produce a list of the includes so we had something to work with.

[arsenm]

Using explicit zOS run line demonstrates the host issue, so I added one of those, but the limits include is enough to fix it. This is still syntax only so the lack of codegen support shouldn't matter

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[perry-ca]

Thanks for the changes. I applied the change to the wrapper to include <climits>. That solved the compile error when running the clang command. I appear to have a single target build as running the test with llvm-lit is giving unsupported as the result. I'll rebuild and try llvm-lit again.

I don't think we need the other change to the .cu file. Won't that run the clang command twice if the user builds clang multi target. I think adding the climits solved the problem.

[arsenm]

Thanks for the changes. I applied the change to the wrapper to include <climits>. That solved the compile error when running the clang command. I appear to have a single target build as running the test with llvm-lit is giving unsupported as the result. I'll rebuild and try llvm-lit again.

It should still be running the one supported run line and skipping the other (plus it still depends on nvptx, so you need at least those 2)

I don't think we need the other change to the .cu file. Won't that run the clang command twice if the user builds clang multi target. I think adding the climits solved the problem.

Yes, it will run twice. It catches the missed limits include from the zos run line, which is better than waiting for a failure on a zos host