This is an archive of the discontinued LLVM Phabricator instance.

[AIX][compiler-rt] Use the AR/ranlib mode flag for 32-bit and 64-bit mode
ClosedPublic

Authored by daltenty on Sep 3 2020, 2:54 PM.

Details

Summary

since we will be building both 32-bit and 64-bit compiler-rt builtins
from a single configuration.

Diff Detail

Event Timeline

daltenty created this revision.Sep 3 2020, 2:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 3 2020, 2:54 PM
Herald added subscribers: Restricted Project, mgorny, dberris. · View Herald Transcript
daltenty requested review of this revision.Sep 3 2020, 2:54 PM
daltenty updated this revision to Diff 289959.Sep 4 2020, 8:19 AM
  • Add AR flags to the builtins CMake too since it may be used as a toplevel by the runtimes build
hubert.reinterpretcast retitled this revision from [AIX][compiler-rt] Use the AR mode flag for 32-bit and 64-bit mode to [AIX][compiler-rt] Use the AR/ranlib mode flag for 32-bit and 64-bit mode.Sep 10 2020, 2:28 PM

since we will be building both 32-bit and 64-bit compiler-rt builtins
from a single configuration.

@daltenty, from the limited context of this patch, this sounds questionable in terms of avoiding silent errors now and in the future. If the idea is to configure once and then run twice in the same build tree, then some things may get cached either in CMake or in generated headers, etc.

Would the 32-bit and 64-bit objects be built into different directories during the CMake build process?

It also seems like CMake needs to be run twice with a different OBJECT_MODE setting in the environment.

compiler-rt/CMakeLists.txt
488–489

Remove extra spaces.

compiler-rt/lib/builtins/CMakeLists.txt
31–32

Remove extra spaces.

since we will be building both 32-bit and 64-bit compiler-rt builtins
from a single configuration.

@daltenty, from the limited context of this patch, this sounds questionable in terms of avoiding silent errors now and in the future. If the idea is to configure once and then run twice in the same build tree, then some things may get cached either in CMake or in generated headers, etc.

Compiler-rt supports generating multiple targets from a single configuration. By default the compiler-rt build process will figure out the supported arches and generate different targets for the powerpc and powerpc64 objects, so there is only one real run.

The problem is that the CMake configuration doesn't really expect to need seperate AR flags for these targets and the environment OBJECT_MODE flag may not reflect what is actually being generated. I agree this is a bit of a blunt solution, but I'm not sure how much value we'd really get from being more specific with the mode flags (and it may hurt us if we want to start doing combined archives)

Would the 32-bit and 64-bit objects be built into different directories during the CMake build process?

That actually depends on the LLVM_ENABLE_PER_TARGET_RUNTIME_DIR setting, see D45604. My assumption was AIX would adopt the traditional single directory layout similar to BSD / Solaris and friends, but I'll be putting up another PR to establish that default so that's definitely open for discussion.

It also seems like CMake needs to be run twice with a different OBJECT_MODE setting in the environment.

As noted above, it seems a single compiler-rt configuration may be expected to build for multiple targets, regardless of the default target / OBJECT_MODE setting.

daltenty updated this revision to Diff 292563.Sep 17 2020, 10:39 AM
  • Fix whitespace and indentation
daltenty marked 2 inline comments as done.Sep 17 2020, 10:40 AM

LGTM; thanks.

Compiler-rt supports generating multiple targets from a single configuration. By default the compiler-rt build process will figure out the supported arches and generate different targets for the powerpc and powerpc64 objects, so there is only one real run.

Okay, I think I get it. With Clang as the build compiler, the target triple encodes the address mode. The compiler driver takes care of the object mode for compiling and linking.

This revision is now accepted and ready to land.Sep 21 2020, 4:54 PM