This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Don't parse target triple except for arch
ClosedPublic

Authored by phosek on Aug 9 2018, 7:02 PM.

Details

Summary

compiler-rt CMake build currently tries to parse the triple and then
put it back together, but doing so inherently tricky, and doing so
from CMake is just crazy and currently doesn't handle triples that
have more than three components. Fortunatelly, the CMake really only
needs the architecture part, which is typically the first component,
to construct variants for other architectures. This means we can keep
the rest of the triple as is and avoid the parsing altogether.

Diff Detail

Event Timeline

phosek created this revision.Aug 9 2018, 7:02 PM
Herald added subscribers: Restricted Project, llvm-commits, dexonsmith and 3 others. · View Herald TranscriptAug 9 2018, 7:02 PM
phosek added inline comments.Aug 9 2018, 10:31 PM
compiler-rt/cmake/Modules/CompilerRTUtils.cmake
279

I'm not sure if COMPILER_RT_DEFAULT_TARGET_TRIPLE_SUFFIX is the best name for what this represents (triple without the arch portion) so I'm open to better suggestions if you have any.

phosek updated this revision to Diff 160167.Aug 10 2018, 12:14 PM
phosek marked an inline comment as done.
phosek added inline comments.Aug 10 2018, 12:49 PM
compiler-rt/cmake/Modules/CompilerRTUtils.cmake
279

I've done a bit more cleanup and this is no longer needed.

morehouse added inline comments.Aug 13 2018, 9:30 AM
compiler-rt/cmake/Modules/CompilerRTUtils.cmake
98

Should this be begin + length - 1 since the foreach loop includes end in its range?

106

Maybe break() after the first index that's too large?

319

Do we need to implement list_sublist if this is the only use of it? Maybe removing the first element from triple_list would be simpler.

phosek updated this revision to Diff 160508.Aug 13 2018, 7:44 PM
phosek marked 3 inline comments as done.
morehouse accepted this revision.Aug 14 2018, 9:14 AM
This revision is now accepted and ready to land.Aug 14 2018, 9:14 AM
This revision was automatically updated to reflect the committed changes.