Page MenuHomePhabricator

[llvm] Add OpenACC as a dependency to llvm-config
AbandonedPublic

Authored by leonardchan on Nov 13 2020, 4:23 PM.

Details

Summary

As of D90848, running ninja check-llvm after a clean cmake configuration will cause these tests to fail:

Failed Tests (2):
  LLVM :: tools/llvm-config/booleans.test
  LLVM :: tools/llvm-config/system-libs.test

from missing lib/libLLVMFrontendOpenACC.a. See https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-linux-x64/b8863755892480463712?.

This just adds LLVMFrontendOpenACC as a dependency to llvm-config so it we be built when building llvm-config.

Diff Detail

Unit TestsFailed

TimeTest
360 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

leonardchan created this revision.Nov 13 2020, 4:23 PM
leonardchan requested review of this revision.Nov 13 2020, 4:23 PM

I know this fixes a bug, but I don't think this is quite correct. It's really only the test that depends on the library not the tool. You could add LLVMFrontendOpenACC as a dependency to the check target, but it's pretty uncommon for an LLVM library to not have any tests. I think it would be better to fix this by adding a test case for LLVMFrontendOpenACC.

I know this fixes a bug, but I don't think this is quite correct.

I agree :)

It's really only the test that depends on the library not the tool.

I am not convinced here: just running bin/llvm-config --shared-mode is broken right now.

I wonder if this library should just not be a component.

I send out https://reviews.llvm.org/D91470 as another take on this.

Is this obsolete after D91470 has been committed?

leonardchan abandoned this revision.Nov 16 2020, 10:34 AM

Is this obsolete after D91470 has been committed?

Yup, closing. Thanks all for the fixes.