Skip to content

Refactored core folder - #298

Open
Hendrixx-RE wants to merge 3 commits into
riscv-software-src:masterfrom
Hendrixx-RE:Hendrixx-RE/issue_3_code_refactor
Open

Refactored core folder #298
Hendrixx-RE wants to merge 3 commits into
riscv-software-src:masterfrom
Hendrixx-RE:Hendrixx-RE/issue_3_code_refactor

Conversation

@Hendrixx-RE

@Hendrixx-RE Hendrixx-RE commented Feb 20, 2026

Copy link
Copy Markdown

Refactor(Core): Decompose Core into Per-Unit Static Libraries addresses #3

This PR addresses the request to refactor the core directory.Each logical unit within core has been organized into its own directory and is now built as a separate static library.

Summary of Changes:

  1. Structural Reorganization:

    • All source files previously in the core root have been moved into specialized subdirectories: cpu, inst, memory, rob, preloader, utils, and
      sim_engine.
  2. Build System Refactoring (CMake):

    • Component Libraries: Each new subdirectory now contains a CMakeLists.txt file that defines its contents as a static library (e.g.,
      libcore_cpu.a, libcore_inst.a).
    • Monolithic Core Removed: The old, single core and instgen library targets have been completely removed.
    • Orchestration: The core/CMakeLists.txt file now serves as an orchestrator, using add_subdirectory() to include all component libraries.
    • Final Linkage: The root CMakeLists.txt has been updated to link the olympia executable and all relevant tests directly against the new granular
      libraries (core_cpu, core_inst, fetch, decode, etc.).
  3. Dependency Resolution:

    • Systematically updated the CMakeLists.txt for all affected components (both new and existing) to explicitly declare their inter-dependencies
      using target_link_libraries. This ensures correct include path propagation and resolves all build errors.

Current Status:

[x] Olympia builds successfully.

[X] Regression tests are working

Feedback Needed:
Is this the intended result?
Also what did you mean by unit testing for each unit?

@pm-ju

pm-ju commented Feb 21, 2026

Copy link
Copy Markdown
Contributor

Can you check all relative address mapping are correct or not, I can see some problems.

@Hendrixx-RE

Hendrixx-RE commented Feb 21, 2026

Copy link
Copy Markdown
Author

Well that's expected,it's a work in progress I just wanted to make sure the implementation is as intended.Can you confirm the same?
If it is I'll complete the pr

@Hendrixx-RE
Hendrixx-RE marked this pull request as ready for review February 22, 2026 12:23
@klingaard

Copy link
Copy Markdown
Collaborator

Yes, this is the intent of the reorganization issue. Appreciate your effort on this.

We did talk about this PR during our last meeting and decided to get the other PRs merged before focusing on this one. Apologies for not commenting earlier.

@Hendrixx-RE Hendrixx-RE reopened this Mar 21, 2026
@Hendrixx-RE

Copy link
Copy Markdown
Author

haha no worries,just let me know any updates on this.Also I dont use ubuntu so I cant verify the regress failing.Its working fine on my arch based os and regress is passing.

@Hendrixx-RE

Copy link
Copy Markdown
Author

Im sorry I didnt check thoroughly for linking errors,I fixed it now kindly review and rerun the tests

@Ma-gi-cian

Copy link
Copy Markdown
Collaborator

@klingaard, I have gone through the entire PR and it looks good from the edm integration point. I aim to have the EDMInstanceGenerator inside the core/inst directory in the InstanceGenerator.hpp/.cpp files. And we could have an edm folder in the core directory, which would hold the code for the Interface and and backends folder inside the edm for the adapters.

@klingaard

Copy link
Copy Markdown
Collaborator

Ok, thanks for looking it over. Let's discuss this PR in the next SIG meeting. If folks feel ok with it (it's quite a transition), then we'll merge it in.

@klingaard

Copy link
Copy Markdown
Collaborator

I'd like to merge this in after the current PRs we have are in.

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.

4 participants