Skip to content
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

Fix LLVM 18 patch for bytecode #42

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

alan-j-hu
Copy link
Contributor

No description provided.

@alan-j-hu
Copy link
Contributor Author

Hold on, don't merge this either yet. Bytecode executables work now, but I made a mistake in the patch.

@terencode
Copy link

You might want to rebase into a single commit.

@alan-j-hu alan-j-hu force-pushed the fix-llvm-bytecode-patch branch from ab7c7d1 to e5d49c1 Compare January 25, 2025 17:51
@alan-j-hu
Copy link
Contributor Author

Okay, fine

@mseri
Copy link
Member

mseri commented Jan 25, 2025

Should I merge this?

@alan-j-hu
Copy link
Contributor Author

@mseri Right now, I'm waiting for the CI to run, but one issue I noticed is that when I remove the LLVM package, the stublibs (in the stublibs directory) are not removed. However, AFAIK if the package is reinstalled, the old stublibs should be overwritten with the new ones.

As far as I know, the previous releases of LLVM on opam to use CMake did not generate stublibs because they passed the -custom flag, but this seems to break bytecode executables.

Is this a problem? Do you happen to know what fields are necessary to get opam to remove the stublibs?

@alan-j-hu
Copy link
Contributor Author

I spoke wrongly, it seems that LLVM 14 does generate stublibs, and those are deleted when it is removed. I'm trying to figure out why the stublibs aren't being removed with this patch.

@alan-j-hu
Copy link
Contributor Author

Nevermind, looks like it was a one-time fluke. There was one time when I removed the LLVM 18 package, and opam outputted an error message about not being able to remove the stublibs. However, I could not reproduce this problem.

Sometimes, opam reports trouble undoing certain changes (such as undoing permission changes to certain files). This seems to be a flaky issue that has to do with opam, not packaging issues. The issue with removing the stublibs was such an error.

@alan-j-hu
Copy link
Contributor Author

I'm doing some testing. I need to test four possibilities:

  • llvm-static, native
  • llvm-static, bytecode
  • llvm-shared, native
  • llvm-shared, bytecode

Now the llvm-static, bytecode combination broke. I suspect I already know the problem; it's a std C++ linking issue that I saw in the original, upstream CMake file.

I wish I had a better way to test this. In particular, these issues are not apparent in the opam CI, which tests installing packages, not building with them. These issues sometimes only become apparent when attempting to run an executable (e.g. a bytecode executable). Therefore, when attempting to make these pull requests, I've undergone a stressful whack-a-mole of fix one issue, another one pops up. Unless there is a CI that tests installing the package, building with it, and running a test executable, I can't have confidence that I didn't miss another issue.

@alan-j-hu
Copy link
Contributor Author

alan-j-hu commented Jan 26, 2025

I'm sorry, I'm feeling very frustrated and stressed right now. I may need to step away from this work to take a breather. Attempting to manage this release is extremely difficult because I need to move between four projects:

  • The llvm-project repo
  • The opam-repository repo
  • The opam-source-archives repo
  • My LLVM test project

and manage four combinations:

  • static, native
  • static, bytecode
  • shared, native
  • shared, bytecode

I need to make sure all these work. (The bytecode wasn't even working, for LLVM 14 release.) I don't have the proper support to do this (e.g. automated testing). Furthermore, I'm going through other stressful life aspects right now. Native compilation seems to work smoothly, but bytecode compilation has been consistently turning up issues.

Over the past few days, I've kept stumbling over mistakes. I think this is a sign that I need to step away from this for a few days, while I clear my head, deal with more urgent things in my life, and eventually figure out if there's a better way to test the LLVM bindings. (I emphasize that the opam CI is not sufficient, as none of these issues become apparent when installing, only when running the bytecode executable.)

@mseri
Copy link
Member

mseri commented Jan 26, 2025

Maybe we could have a test building and running a simple bytecode executable so that we may catch something with the CI. Not sure if that helps, I understand that it is quite a delicate and complex problem and setup

@alan-j-hu
Copy link
Contributor Author

alan-j-hu commented Jan 28, 2025

@mseri

After hitting my head against a wall in frustration for more than a day, trying to figure out what was wrong with my CMake code, I decided to install and test the LLVM 14 bindings, released on opam. I was basing my own work off of the LLVM 14 CMake patch files in this repository, which I assumed were correct.

It seems that LLVM 14, statically linking to LLVM, does not work. Given the following demo code:

let () =
  let context = Llvm.global_context () in
  let module_ = Llvm.create_module context "module" in
  Llvm.dump_module module_

The following works:

$ ocamlfind ocamlopt -package llvm -linkpkg -o test test.ml
$ ./test
; ModuleID = 'module'
source_filename = "module"

However, the following (adding -predicates llvm.static) does not work:

$ ocamlfind ocamlopt -predicates llvm.static -package llvm -linkpkg -o test test.ml
/usr/bin/ld: /usr/lib/llvm-14/lib/libLLVMCore.a(Dominators.cpp.o): in function `llvm::DominatorTreeBase<llvm::BasicBlock, false>::DominatorTreeBase(llvm::DominatorTreeBase<llvm::BasicBlock, false>&&)':
(.text._ZN4llvm17DominatorTreeBaseINS_10BasicBlockELb0EEC2EOS2_[_ZN4llvm17DominatorTreeBaseINS_10BasicBlockELb0EEC5EOS2_]+0xf4): undefined reference to `operator delete(void*)'
/usr/bin/ld: /usr/lib/llvm-14/lib/libLLVMCore.a(Dominators.cpp.o): in function `llvm::DominatorTreeBase<llvm::BasicBlock, false>::wipe()':
(.text._ZN4llvm17DominatorTreeBaseINS_10BasicBlockELb0EE4wipeEv[_ZN4llvm17DominatorTreeBaseINS_10BasicBlockELb0EE4wipeEv]+0x54): undefined reference to `operator delete(void*)'
/usr/bin/ld: /usr/lib/llvm-14/lib/libLLVMCore.a(Dominators.cpp.o): in function `llvm::DominatorTreeBase<llvm::BasicBlock, false>::operator=(llvm::DominatorTreeBase<llvm::BasicBlock, false>&&)':
(.text._ZN4llvm17DominatorTreeBaseINS_10BasicBlockELb0EEaSEOS2_[_ZN4llvm17DominatorTreeBaseINS_10BasicBlockELb0EEaSEOS2_]+0xa4): undefined reference to `operator delete(void*)'
/usr/bin/ld: /usr/lib/llvm-14/lib/libLLVMCore.a(Dominators.cpp.o): in function `llvm::DenseMap<llvm::BasicBlock*, std::unique_ptr<llvm::DomTreeNodeBase<llvm::BasicBlock>, std::default_delete<llvm::DomTreeNodeBase<llvm::BasicBlock> > >, llvm::DenseMapInfo<llvm::BasicBlock*, void>, llvm::detail::DenseMapPair<llvm::BasicBlock*, std::unique_ptr<llvm::DomTreeNodeBase<llvm::BasicBlock>, std::default_delete<llvm::DomTreeNodeBase<llvm::BasicBlock> > > > >::operator=(llvm::DenseMap<llvm::BasicBlock*, std::unique_ptr<llvm::DomTreeNodeBase<llvm::BasicBlock>, std::default_delete<llvm::DomTreeNodeBase<llvm::BasicBlock> > >, llvm::DenseMapInfo<llvm::BasicBlock*, void>, llvm::detail::DenseMapPair<llvm::BasicBlock*, std::unique_ptr<llvm::DomTreeNodeBase<llvm::BasicBlock>, std::default_delete<llvm::DomTreeNodeBase<llvm::BasicBlock> > > > >&&)':
(.text._ZN4llvm8DenseMapIPNS_10BasicBlockESt10unique_ptrINS_15DomTreeNodeBaseIS1_EESt14default_deleteIS5_EENS_12DenseMapInfoIS2_vEENS_6detail12DenseMapPairIS2_S8_EEEaSEOSE_[_ZN4llvm8DenseMapIPNS_10BasicBlockESt10unique_ptrINS_15DomTreeNodeBaseIS1_EESt14default_deleteIS5_EENS_12DenseMapInfoIS2_vEENS_6detail12DenseMapPairIS2_S8_EEEaSEOSE_]+0x34): undefined reference to `operator delete(void*)'
/usr/bin/ld: /usr/lib/llvm-14/lib/libLLVMCore.a(Dominators.cpp.o): in function `llvm::DominatorTreeBase<llvm::BasicBlock, false>::createChild(llvm::BasicBlock*, llvm::DomTreeNodeBase<llvm::BasicBlock>*)':
(.text._ZN4llvm17DominatorTreeBaseINS_10BasicBlockELb0EE11createChildEPS1_PNS_15DomTreeNodeBaseIS1_EE[_ZN4llvm17DominatorTreeBaseINS_10BasicBlockELb0EE11createChildEPS1_PNS_15DomTreeNodeBaseIS1_EE]+0x2d): undefined reference to `operator new(unsigned long)'
/usr/bin/ld: (.text._ZN4llvm17DominatorTreeBaseINS_10BasicBlockELb0EE11createChildEPS1_PNS_15DomTreeNodeBaseIS1_EE[_ZN4llvm17DominatorTreeBaseINS_10BasicBlockELb0EE11createChildEPS1_PNS_15DomTreeNodeBaseIS1_EE]+0x11b): undefined reference to `operator delete(void*)'

and error messages that look like this continue down the screen.

To emphasize, it appears that the published version of LLVM 14 on opam is broken. Can you confirm that you can also reproduce this issue when compiling with the -predicates llvm.static flag?

I was able to get both native and bytecode working when using the LLVM dynamic library. Right now, the issue seems to be compiling with the LLVM static library. I am experiencing difficulty figuring out the cause of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants