-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[deps/llvm] don't use hardcode LLVM_SHARED_LIB_NAME
#45908
Conversation
Thank you for picking this up! |
# import LLVM_SHARED_LIB_NAME | ||
include $(JULIAHOME)/deps/llvm-ver.make |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should not depend on deps/llvm-ver.make
, since the correct value is given by llvm-config-host --version
, while llvm-ver.make
might not have version info (either outdated, wrong, or not available from git)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vtjnash But We do not call llvm-config-host
here.
We only need LLVM_SHARED_LIB_NAME
in main Makefile.
LLVM_SHARED_LIB_NAME
<- LLVM_SHARED_LIB_VER_SUFFIX
<- LLVM_VER_MAJ
<- LLVM_VER
LLVM_VER
: A string, updated manually.
Line 6 in c5aa255
LLVM_VER := 14.0.5 LLVM_VER_MAJ
: generated by Makefile string substitute
Line 3 in c5aa255
LLVM_VER_MAJ:=$(word 1, $(subst ., ,$(LLVM_VER))) LLVM_SHARED_LIB_VER_SUFFIX
: string concat
Line 16 in c5aa255
LLVM_SHARED_LIB_VER_SUFFIX := $(LLVM_VER_MAJ)jl LLVM_SHARED_LIB_NAME
: string concat
Line 18 in c5aa255
LLVM_SHARED_LIB_NAME := libLLVM-$(LLVM_SHARED_LIB_VER_SUFFIX)
The original shared library name is a fixed string. It needs to be updated manually.
So my patch is mainly to keep the version in the lib name in sync with LLVM_VER
.
If necessary I can also create a new variable and call llvm-config-host
to get the correct parameters.
(cherry picked from commit eb72c2a)
(cherry picked from commit eb72c2a)
Hand edited changes to deps/llvm.version and deps/Versions.make Moved LLVM_VER from Versions.make to llvm.version by Tim S. (cherry picked from commit eb72c2a)
Hand edited changes to deps/llvm.version and deps/Versions.make Moved LLVM_VER from Versions.make to llvm.version by Tim S. (cherry picked from commit eb72c2a)
Hand edited changes to deps/llvm.version and deps/Versions.make Moved LLVM_VER from Versions.make to llvm.version by Tim S. (cherry picked from commit eb72c2a)
Hand edited changes to deps/llvm.version and deps/Versions.make Moved LLVM_VER from Versions.make to llvm.version by Tim S. (cherry picked from commit eb72c2a) Co-authored-by: woclass <git@wo-class.cn>
This pr will replace #44260
$(LLVM_CONFIG_HOST) --libs --link-shared
is still broken on Windows.Related updtream issues: llvm/llvm-project#55544, llvm/llvm-project#39599