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

removed memories and changed purposes of some submodules #817

Merged
merged 8 commits into from
Mar 22, 2024

Conversation

Vasco-Luz
Copy link
Contributor

Removed memories and changed the purpose of cache and rams so the sources are copied to fpga/src and simulation/src.
changed the simulation and fpga makefile so this simulation/src/.v or fpga/src/.v are included. Changed something in iob_cache.py so now the .v that shold be delected now are in fpga and in simulation instead of hardware/src.

iob_soc.py Outdated Show resolved Hide resolved
if "_ram_" in cls.name or "_rom_" in cls.name:
dst_directory = cls.get_purpose_dir(cls.get_setup_purpose())
dst_directory = cls.get_purpose_dir(cls.get_setup_purpose())
if cls.use_netlist:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

back to the original iob_module.py

Copy link
Contributor

@P-Miranda P-Miranda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also found the problem with iob_cache setup as one of first submodules:
the problem is in iob_utils.py, change the original code:

                # Delete sources for this purpose
                os.remove(
                    os.path.join(
                        cls.build_dir, cls.cls.PURPOSE_DIRS[purpose], "iob_utils.vh" # cls.cls is wrong
                    )
                )

into this:

                os.remove(
                    os.path.join(
                        cls.build_dir, cls.PURPOSE_DIRS[purpose], "iob_utils.vh"  # THIS LINE
                    )
                )

@@ -128,6 +127,8 @@ def _create_submodules_list(cls, extra_submodules=[]):
(iob_tasks, {"purpose": "simulation"}),
# Software modules
printf,
(iob_cache, {"purpose": "simulation"}),
(iob_cache, {"purpose": "fpga"}),
# Modules required for CACHE
Copy link
Contributor

@P-Miranda P-Miranda Mar 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand why we have to do this:
the cache does not add memories for hardware purpose, only for simulation or fpga
therefore, setting up cache as submodule, does not add the memories
and we have to add the memories in the iob_soc submodule list as well

one option would be removing this from iob_module.py:

            # Don't setup submodules that have a purpose different than
            # "hardware" when this class is not the top module
            if not cls.is_top_module and setup_options["purpose"] != "hardware":
                continue

or create a new memory purpose to signal that the submodule should be setup even if is not top module, but otherwise are treated as simulation and fpga purposes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change iob_soc.py to not add cache if USE_EXTMEM=0

is there anything else to this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question havin to do with rams/roms, should they have their purpose fixed at fpga and simulation?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

submodules/LIB/hardware/fpga/Makefile Outdated Show resolved Hide resolved
submodules/LIB/hardware/simulation/Makefile Outdated Show resolved Hide resolved
submodules/LIB/hardware/fpga/Makefile Outdated Show resolved Hide resolved
submodules/LIB/hardware/simulation/Makefile Outdated Show resolved Hide resolved
@@ -27,6 +27,6 @@ def _copy_srcs(cls):
# Delete sources for this purpose
os.remove(
os.path.join(
cls.build_dir, cls.PURPOSE_DIRS[purpose], "iob_utils.vh"
Copy link
Contributor Author

@Vasco-Luz Vasco-Luz Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not understanding why git is flagging this as a change. the change was done 4 commits ago.

iob_soc.py Outdated
Comment on lines 141 to 142
if "USE_EXTMEM" not in sys.argv:
submodule_list.remove(iob_cache)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of adding cache and then removing,
only add cache and cache memory submodujles if necessary:

if "USE_EXTMEM" in sys.argv:
    submodule_list += [
        iob_cache,
        # Modules required for CACHE
        (iob_ram_2p, {"purpose": "simulation"}),
        (iob_ram_2p, {"purpose": "fpga"}),
        (iob_ram_sp, {"purpose": "simulation"}),
        (iob_ram_sp, {"purpose": "fpga"}),
    ]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change made

@@ -128,6 +127,8 @@ def _create_submodules_list(cls, extra_submodules=[]):
(iob_tasks, {"purpose": "simulation"}),
# Software modules
printf,
(iob_cache, {"purpose": "simulation"}),
(iob_cache, {"purpose": "fpga"}),
# Modules required for CACHE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

@jjts jjts merged commit db55b6a into IObundle:main Mar 22, 2024
8 checks passed
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