Page MenuHomePhabricator

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

Authored by jhuber6 on Oct 19 2020, 6:24 AM.

Details

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.

Diff Detail

Unit TestsFailed

TimeTest
400 mslinux > HWAddressSanitizer-x86_64.TestCases::sizes.cpp
Script: -- : 'RUN: at line 3'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -m64 -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -mcmodel=large -mllvm -hwasan-globals -mllvm -hwasan-use-short-granules -mllvm -hwasan-instrument-landing-pads=0 -mllvm -hwasan-instrument-personality-functions /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/hwasan/TestCases/sizes.cpp -nostdlib++ -lstdc++ -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/hwasan/X86_64/TestCases/Output/sizes.cpp.tmp

Event Timeline

jhuber6 created this revision.Oct 19 2020, 6:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 19 2020, 6:24 AM
jhuber6 requested review of this revision.Oct 19 2020, 6:24 AM
This revision is now accepted and ready to land.Oct 19 2020, 7:52 AM
jhuber6 updated this revision to Diff 299081.Oct 19 2020, 9:26 AM

Checking tests again.

This revision was automatically updated to reflect the committed changes.
daltenty added inline comments.
clang/test/OpenMP/driver-openmp-target.c
2 ↗(On Diff #299103)

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 added inline comments.Oct 20 2020, 4:36 PM
clang/test/OpenMP/driver-openmp-target.c
2 ↗(On Diff #299103)

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 updated this revision to Diff 299672.Oct 21 2020, 6:57 AM

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.

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

@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
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.

174

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

@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 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.