This is an archive of the discontinued LLVM Phabricator instance.

[Clang] Fix clang_rt tests when LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is ON
ClosedPublic

Authored by michaelplatings on Feb 9 2023, 10:17 AM.

Details

Summary

If clang is part of a toolchain that can locate clang_rt libraries
outside its resource directory and these are built with
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON then the tests would fail because
the library names don't have the arch suffix. This change makes the arch
suffix optional.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2023, 10:17 AM
michaelplatings requested review of this revision.Feb 9 2023, 10:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2023, 10:17 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
simon_tatham accepted this revision.Feb 21 2023, 1:14 AM
This revision is now accepted and ready to land.Feb 21 2023, 1:14 AM

I missed this change so I apologize for late response, but can you elaborate on why this is needed? Clang first checks the name without architecture and if the file doesn't exist then it'll return the one with architecture unconditionally, see https://github.com/llvm/llvm-project/blob/d61a863050bb4afd22d08bbe53af1e24c0657aba/clang/lib/Driver/ToolChain.cpp#L545. Since there are no files inside https://github.com/llvm/llvm-project/tree/main/clang/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/arm, it should always return the path with architecture as was the case for the previous version. LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON should play no role here since Clang tests are hermetic (as in they don't rely on runtimes being built).

Hi Petr,
In LLVM Embedded Toolchain for Arm we put libclang_rt in the regular library directory, not the resource dir. Therefore -resource-dir has no effect for us.
Instead of this change, I could have added a --sysroot argument to each test, but this change seemed to fit well with the discussion in https://discourse.llvm.org/t/rfc-time-to-drop-legacy-runtime-paths/64628.

I'm happy to revert this and add --sysroot if you prefer. That would fit with keeping the tests hermetic.

I'm happy to revert this and add --sysroot if you prefer. That would fit with keeping the tests hermetic.

That would be my preference and it would match what we do in other Clang tests.