[OpenMP] Fixing OpenMP/driver.c failing on 32-bit hosts

[jhuber6]

Summary

The changes made in D88594 caused the test OpenMP/driver.c to fail on a 32-bit host becuase it was offloading to a 64-bit architecture by default. The offloading test was moved to a new file and a feature was added to the lit config to check for a 64-bit host.

[jdoerfert]

LGTM

[jhuber6]

Checking tests again.

[daltenty]

clang/test/OpenMP/driver-openmp-target.c

3

This tests still fails on AIX, presumably because we have a 64-bit host but the compiler is targeting 32-bit by default:

error: OpenMP target architecture 'x86_64-unknown-unknown' pointer size is incompatible with host 'powerpc-ibm-aix7.2.0.0'

Maybe adjust the test to check the default target instead of just the host?

[jhuber6]

clang/test/OpenMP/driver-openmp-target.c

3

I'm not aware of any easy methods to do that, though one might exist. Easiest solution is just add a check that doesn't run this test on AIX, more complex would probably require taking the default target from the clang executable and parsing it using the same methods that Triple.cpp uses to get the architecture size. I'm not familiar with anything about AIX, any suggestions?

[jhuber6]

Changing test to be similar to LLVM's method, checks the linker flags for 32-bit library and only if the target triple is one of the known 64-bit triples.

[jhuber6]

@daltenty Do you think this will fix the problem on AIX?

[daltenty]

In D89696#2344508, @jhuber6 wrote:

@daltenty Do you think this will fix the problem on AIX?

It's not just AIX that will have this problem I suspect. If you configure any cross-compiling build with a 64-bit host, targeting a 32-bit arch, your going to run into the problem. I think checking against a list of known 64-bit arches is good enough for the purposes here, but the check still needs to be against the target, not the host.

Also, the frontend diagnostic looks like it is off for the same reason:

else if (getArchPtrSize(T) != getArchPtrSize(TT))
  Diags.Report(diag::err_drv_incompatible_omp_arch)

T seems to be the target triple, not the host, but the diagnostic reads: pointer size is incompatible with host.

clang/test/lit.cfg.py

170–175

Maybe this should be something like clang-64-bit-default-target instead to clarify what we mean.

172

I don't think this check is meaningful. This just checks how llvm was built, it doesn't tell you anything about what clang driver is going to target out of the box, which is what we care about here.

[jhuber6]

In D89696#2344753, @daltenty wrote:

In D89696#2344508, @jhuber6 wrote:

@daltenty Do you think this will fix the problem on AIX?

It's not just AIX that will have this problem I suspect. If you configure any cross-compiling build with a 64-bit host, targeting a 32-bit arch, your going to run into the problem. I think checking against a list of known 64-bit arches is good enough for the purposes here, but the check still needs to be against the target, not the host.

Also, the frontend diagnostic looks like it is off for the same reason:

else if (getArchPtrSize(T) != getArchPtrSize(TT))
  Diags.Report(diag::err_drv_incompatible_omp_arch)

T seems to be the target triple, not the host, but the diagnostic reads: pointer size is incompatible with host.

TT is the target triple, otherwise the error message would be backwards, x86 is the OpenMP target architecture and PPC is the host architecture. I'm not sure what you mean by checking the target rather than the host. The target is set explicitly by -fopenmp-targets=<triple>, since it's being set in the test file we try to make sure that the host matches it. If this was just a frontend test we could just specify the host Triple and be done with it. Maybe we could add some CMake options for which offloading libraries were built?

[daltenty]

In D89696#2344797, @jhuber6 wrote:

In D89696#2344753, @daltenty wrote:

In D89696#2344508, @jhuber6 wrote:

@daltenty Do you think this will fix the problem on AIX?

It's not just AIX that will have this problem I suspect. If you configure any cross-compiling build with a 64-bit host, targeting a 32-bit arch, your going to run into the problem. I think checking against a list of known 64-bit arches is good enough for the purposes here, but the check still needs to be against the target, not the host.

Also, the frontend diagnostic looks like it is off for the same reason:

else if (getArchPtrSize(T) != getArchPtrSize(TT))
  Diags.Report(diag::err_drv_incompatible_omp_arch)

T seems to be the target triple, not the host, but the diagnostic reads: pointer size is incompatible with host.

TT is the target triple, otherwise the error message would be backwards, x86 is the OpenMP target architecture and PPC is the host architecture. I'm not sure what you mean by checking the target rather than the host. The target is set explicitly by -fopenmp-targets=<triple>, since it's being set in the test file we try to make sure that the host matches it. If this was just a frontend test we could just specify the host Triple and be done with it. Maybe we could add some CMake options for which offloading libraries were built?

Sorry I think I understand what's maybe happening here. I guess the OpenMP terminology here doesn't mesh with really well with the LLVM usages. The term "host" seems to be overloaded to mean two different things, either the OpenMP host target arch ("OpenMP host") or the arch LLVM was compiled for ("LLVM host triple").

So in this case, with AIX we have:

The OpenMP target from commandine is x86
The architecture clang/llvm is compiled for is PPC64 ("LLVM host triple")
The architecture the frontend is set to emit for/target is PPC32 ("OpenMP host architecture", "LLVM target triple")

(This is also what makes the diagnostic confusing, is it talking about the OpenMP host or the compiler host?)

The lit infrastructure uses the LLVM meaning, so config.host_triple is the compiler host, not the LLVM target / OpenMP host architecture. That is what I mean when I say "check the target instead", I mean check the LLVM target instead of the LLVM host triple.

clang/test/OpenMP/driver-openmp-target.c

+// REQUIRES: x86-registered-target
+// REQUIRES: clang-64-bits
+// RUN: %clang %s -c -E -dM -fopenmp=libomp -fopenmp-version=45 -fopenmp-targets=x86_64-unknown-unknown -o - | FileCheck --check-prefix=CHECK-45-VERSION --check-prefix=CHECK-45-VERSION2 %s
+// CHECK-45-VERSION: #define _OPENMP 201511
+// CHECK-45-VERSION2: #define _OPENMP 201511

clang/test/OpenMP/driver.c

 // RUN: %clang %s -c -E -dM -fopenmp=libomp -fopenmp-version=31 | FileCheck --check-prefix=CHECK-31-VERSION %s
 // CHECK-31-VERSION: #define _OPENMP 201107
 
 // RUN: %clang %s -c -E -dM -fopenmp=libomp -fopenmp-version=40 | FileCheck --check-prefix=CHECK-40-VERSION %s
 // CHECK-40-VERSION: #define _OPENMP 201307
 
 // RUN: %clang %s -c -E -dM -fopenmp=libomp -fopenmp-version=45 | FileCheck --check-prefix=CHECK-45-VERSION %s
 // RUN: %clang %s -c -E -dM -fopenmp=libomp -fopenmp-version=45 -fopenmp-simd | FileCheck --check-prefix=CHECK-45-VERSION %s
-// RUN: %clang %s -c -E -dM -fopenmp=libomp -fopenmp-version=45 -fopenmp-targets=x86_64-unknown-unknown -o - | FileCheck --check-prefix=CHECK-45-VERSION --check-prefix=CHECK-45-VERSION2 %s
 // CHECK-45-VERSION: #define _OPENMP 201511
 // CHECK-45-VERSION2: #define _OPENMP 201511
 
 // RUN: %clang %s -c -E -dM -fopenmp=libomp -fopenmp-version=50 | FileCheck --check-prefix=CHECK-50-VERSION %s
 // CHECK-50-VERSION: #define _OPENMP 201811
 
 // RUN: %clang %s -c -E -dM -fopenmp-version=1 | FileCheck --check-prefix=CHECK-VERSION %s
 // RUN: %clang %s -c -E -dM -fopenmp-version=31 | FileCheck --check-prefix=CHECK-VERSION %s

clang/test/lit.cfg.py

 # -*- Python -*-
 
 import os
+import sys
 import platform
 import re
 import subprocess
 import tempfile
 
 import lit.formats
 import lit.util
 

 # Tests that are specific to the Apple Silicon macOS.
 if re.match(r'^arm64(e)?-apple-(macos|darwin)', config.target_triple):
     config.available_features.add('apple-silicon-mac')
 
 # [PR18856] Depends to remove opened file. On win32, a file could be removed
 # only if all handles were closed.
 if platform.system() not in ['Windows']:
     config.available_features.add('can-remove-opened-file')
 
+# Check 64-bit host
+if sys.maxsize > 2**32:
+  config.available_features.add("clang-64-bits")
+
 
 def calculate_arch_features(arch_string):
     features = []
     for arch in arch_string.split():
         features.append(arch.lower() + '-registered-target')
     return features
 
 
 llvm_config.feature_config(