Skip to content

Conversation

@towoe
Copy link
Collaborator

@towoe towoe commented Dec 16, 2025

@widlarizer, I reused some of your commits (also merged a few things together) from #12
My goal would be to bring this up-to-date, and then also have the changes in CIRCT which we also want to have upstream, and maybe leave out the "experimental" things.
So updating the build in here, and then follow up with the features in another PR.

towoe and others added 7 commits December 5, 2025 13:36
Don't limit the parallel setting in the configuration. Let the build
system take care of the optimal parallel jobs.
Make build.sh ninja and cmake commands run in top level working
directory.
Let build.sh fail when a subcommand fails.
@towoe
Copy link
Collaborator Author

towoe commented Dec 16, 2025

I think the things missing from #12 are the attributes and the bytecode output @widlarizer?

@widlarizer
Copy link

Sorry, I don't understand the plan. Is this #12 rebased but you removed some things so they're missing now?

@towoe
Copy link
Collaborator Author

towoe commented Dec 16, 2025

Basically yes, this is now only including the stuff for the build system, to get this updated.
I think there are some other things in PR #12 which might need some more changes, but so make this a bit easier I wanted to do unrelated things in #13. Does that sound okay to you?

@widlarizer
Copy link

Right, so the CIRCT bump includes none of the actual CIRCT changes, I'll see when I can get to rebase all that then

@towoe
Copy link
Collaborator Author

towoe commented Dec 17, 2025

The CIRCT bump should include the existing RTLIL stuff with some rebase adoptions, but none of the change from you fork.
@widlarizer what do you mean with "rebase all that"?

@widlarizer
Copy link

I see, thanks. Is this PR buildable as-is? I tried moving all of #12 on top of this PR but ran into:

CMake Error at llvm/build/lib/cmake/llvm/AddLLVM.cmake:1133 (add_dependencies):
  The dependency target "MLIRRegisterAllDialects" of target
  "circt-lsp-server" does not exist.
Call Stack (most recent call first):
  cmake/modules/AddCIRCT.cmake:50 (add_llvm_executable)
  cmake/modules/AddCIRCT.cmake:59 (add_circt_executable)
  tools/circt-lsp-server/CMakeLists.txt:44 (add_circt_tool)

Which seems quite unrelated to any of the changes I did. But also I may have messed up the rebase. You can look at that rebase here

@towoe
Copy link
Collaborator Author

towoe commented Dec 18, 2025

I tested the PR before I opened it. It should work...
Your error looks suspiciously like an outdated dependency.
Could you try a clean build? With both, circt and llvm, submodules updated.

@widlarizer
Copy link

I rechecked without any of my changes. Even if I have no nix flake bump, with this PR as-is, I still get the same build error when clean building. I don't know what's up with that yet

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