-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
Add symbol visibility macros to abi-breaking.h.cmake #110898
Conversation
Annotating these symbols will fix missing symbols errors for Bugpoint when when the default symbol visibility is set to hidden for LLVM. This is part of the work to enable LLVM_BUILD_LLVM_DYLIB and plugins on window.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/18/builds/5136 Here is the relevant piece of the build log for the reference
|
I guess we will need an lldb expert to decipher that test. @shafik, do you have a clue what might be going wrong here? |
Looks like this was temporary instability. The next build was successful: https://lab.llvm.org/buildbot/#/builders/18/builds/5137 |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/1345 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/64/builds/1186 Here is the relevant piece of the build log for the reference
|
I just found this actually introduce a cyclic module dependency:
abi-breaking.h -> Compiler.h -> lllvm-config.h abi-breaking and llvm-config belong to the same module. |
I wonder how best to fix it, would excluding Compiler.h from the LLVM_Utils module work? |
Maybe we need to break apart LLVM_Config. I don't have other better idea. |
We should probably make Compiler.h textual. |
Any chance for a fix for this? The cyclic dependency is unfortunately breaking our build bots. |
I don't mind if you revert it for now until we get the right fix for it. |
Was that tried, @juliannagele? |
This reverts commit 3be6916.
Reverts #110898 This change has caused a cyclic module dependency `fatal error: cyclic dependency in module 'LLVM_Utils': LLVM_Utils -> LLVM_Config_ABI_Breaking -> LLVM_Utils`. Reverting for now until we the right fix.
👍 reverted for now |
I wouldn't know I'm afraid, happy to give it a try if you have a patch. |
diff --git a/llvm/include/module.modulemap b/llvm/include/module.modulemap
index b00da6d7cd28..0c05e4cf674d 100644
--- a/llvm/include/module.modulemap
+++ b/llvm/include/module.modulemap
@@ -389,7 +389,7 @@ module LLVM_Utils {
exclude header "llvm/Support/PluginLoader.h"
exclude header "llvm/Support/Solaris/sys/regset.h"
textual header "llvm/Support/TargetOpcodes.def"
-
+ textual header "llvm/Support/Compiler.h"
}
module TargetParser { |
Thanks, afaict that would fix the cyclic dependency (then exposing another issue that looks unrelated). |
That's great! Can you revert the revert and insert that fix instead? |
…18464) Reverts llvm#110898 This change has caused a cyclic module dependency `fatal error: cyclic dependency in module 'LLVM_Utils': LLVM_Utils -> LLVM_Config_ABI_Breaking -> LLVM_Utils`. Reverting for now until we the right fix.
Annotating these symbols will fix missing symbols errors for Bugpoint when when the default symbol visibility is set to hidden for LLVM.
This is part of the work to enable LLVM_BUILD_LLVM_DYLIB and plugins on window.