This is an archive of the discontinued LLVM Phabricator instance.

[clang] Replace BACKEND_PACKAGE_STRING with LLVM_VERSION_STRING
ClosedPublic

Authored by MaskRay on Oct 24 2022, 9:03 PM.

Details

Summary

420d7ccbac0f499a6ff9595bdbfa99cd3376df22 introduced BACKEND_PACKAGE_STRING to
replace PACKAGE_VERSION (llvm/Config/config.h) to support standalone builds.
This is used in the output of clang -cc1 -v.

Since llvm-config.h is available for both standalone and non-standalone builds,
we can just use LLVM_VERSION_STRING from llvm-config.h.

clang/cmake/modules/AddClang.cmake uses VERSION_STRING "${CLANG_VERSION} (${BACKEND_PACKAGE_STRING})".
Just simplify it to "${CLANG_VERSION}" so that we can remove the CMake
variable BACKEND_PACKAGE_STRING.

Diff Detail

Event Timeline

MaskRay created this revision.Oct 24 2022, 9:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 24 2022, 9:03 PM
Herald added a subscriber: StephenFan. · View Herald Transcript
MaskRay requested review of this revision.Oct 24 2022, 9:03 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 24 2022, 9:03 PM
MaskRay edited the summary of this revision. (Show Details)Oct 24 2022, 9:06 PM
tstellar accepted this revision.Oct 24 2022, 9:08 PM

This seems fine to me.

This revision is now accepted and ready to land.Oct 24 2022, 9:08 PM

I suppose you want to remove it from clang/CMakeLists.txt as well. I see Flang has copied the same logic too, so might be worthwhile to remove it there as well.

I suppose you want to remove it from clang/CMakeLists.txt as well. I see Flang has copied the same logic too, so might be worthwhile to remove it there as well.

set(BACKEND_PACKAGE_STRING "LLVM ${LLVM_PACKAGE_VERSION}") and another set in clang/CMakeLists.txt is used by clang/cmake/modules/AddClang.cmake.
Perhaps just simplify VERSION_STRING "${CLANG_VERSION} (${BACKEND_PACKAGE_STRING})" to VERSION_STRING "${CLANG_VERSION}" ? The backend package string is probably not useful.

MaskRay updated this revision to Diff 470383.Oct 24 2022, 11:24 PM
MaskRay edited the summary of this revision. (Show Details)

Remove cmake variable BACKEND_PACKAGE_STRING

Since the patch subject is called [clang] I am deferring the flang cmake change to a separate patch.

I suppose you want to remove it from clang/CMakeLists.txt as well. I see Flang has copied the same logic too, so might be worthwhile to remove it there as well.

set(BACKEND_PACKAGE_STRING "LLVM ${LLVM_PACKAGE_VERSION}") and another set in clang/CMakeLists.txt is used by clang/cmake/modules/AddClang.cmake.
Perhaps just simplify VERSION_STRING "${CLANG_VERSION} (${BACKEND_PACKAGE_STRING})" to VERSION_STRING "${CLANG_VERSION}" ? The backend package string is probably not useful.

Perhaps it was used to account for LLVM and Clang minor version mismatch — though I don't know if anyone is actually building a configuration like that.

I suppose you want to remove it from clang/CMakeLists.txt as well. I see Flang has copied the same logic too, so might be worthwhile to remove it there as well.

set(BACKEND_PACKAGE_STRING "LLVM ${LLVM_PACKAGE_VERSION}") and another set in clang/CMakeLists.txt is used by clang/cmake/modules/AddClang.cmake.
Perhaps just simplify VERSION_STRING "${CLANG_VERSION} (${BACKEND_PACKAGE_STRING})" to VERSION_STRING "${CLANG_VERSION}" ? The backend package string is probably not useful.

Perhaps it was used to account for LLVM and Clang minor version mismatch — though I don't know if anyone is actually building a configuration like that.

Yes, and that's used in a minor place for a Windows build... The LLVM version should really not matter.