MiSTer-devel / Arcade-IremM92_MiSTer

GNU General Public License v2.0
17 stars 4 forks source link

Optimize/merge v30 CPUs #9

Closed gyurco closed 1 year ago

gyurco commented 1 year ago

Hi!

I ported your core to MiST, however as it is huge, I had to shrink the biggest part - the CPUs. I cannot test on MiSTer, but it works well on MiST. There's still a potential optimization target - the internal RAM in V35, however it seems the sound boards' code are not using it at all, so I just deleted it (not included in this patch).

birdybro commented 1 year ago

rbfs.zip for test

Left = current core right = gyurco's changes

image

Even slightly improved the timing, potentially (less registers will do that typically, but just different seeding could do that too).

Also noted this from the logs:

Warning (10645): VHDL type inferencing warning at cpu.vhd(2563): two visible identifiers match "TO_INTEGER" because the actual at position 0 has an ambiguous type - it could be "UNSIGNED" or "SIGNED", assuming "UNSIGNED"
Warning (10645): VHDL type inferencing warning at cpu.vhd(2600): two visible identifiers match "TO_INTEGER" because the actual at position 0 has an ambiguous type - it could be "UNSIGNED" or "SIGNED", assuming "UNSIGNED"
Warning (10645): VHDL type inferencing warning at cpu.vhd(2602): two visible identifiers match "TO_INTEGER" because the actual at position 0 has an ambiguous type - it could be "UNSIGNED" or "SIGNED", assuming "UNSIGNED"
Warning (10645): VHDL type inferencing warning at cpu.vhd(2682): two visible identifiers match "TO_INTEGER" because the actual at position 0 has an ambiguous type - it could be "UNSIGNED" or "SIGNED", assuming "UNSIGNED"
Warning (10645): VHDL type inferencing warning at cpu.vhd(2684): two visible identifiers match "TO_INTEGER" because the actual at position 0 has an ambiguous type - it could be "UNSIGNED" or "SIGNED", assuming "UNSIGNED"
Warning (10645): VHDL type inferencing warning at cpu.vhd(2742): two visible identifiers match "TO_INTEGER" because the actual at position 0 has an ambiguous type - it could be "UNSIGNED" or "SIGNED", assuming "UNSIGNED"
Warning (10645): VHDL type inferencing warning at cpu.vhd(2744): two visible identifiers match "TO_INTEGER" because the actual at position 0 has an ambiguous type - it could be "UNSIGNED" or "SIGNED", assuming "UNSIGNED"

Referring to these lines:

https://github.com/gyurco/Arcade-IremM92_MiSTer/blob/700bcbad15e3caace61580b50b394284896b499b/rtl/v30/cpu.vhd#L2563

https://github.com/gyurco/Arcade-IremM92_MiSTer/blob/700bcbad15e3caace61580b50b394284896b499b/rtl/v30/cpu.vhd#L2600

https://github.com/gyurco/Arcade-IremM92_MiSTer/blob/700bcbad15e3caace61580b50b394284896b499b/rtl/v30/cpu.vhd#L2602

https://github.com/gyurco/Arcade-IremM92_MiSTer/blob/700bcbad15e3caace61580b50b394284896b499b/rtl/v30/cpu.vhd#L2682

https://github.com/gyurco/Arcade-IremM92_MiSTer/blob/700bcbad15e3caace61580b50b394284896b499b/rtl/v30/cpu.vhd#L2684

https://github.com/gyurco/Arcade-IremM92_MiSTer/blob/700bcbad15e3caace61580b50b394284896b499b/rtl/v30/cpu.vhd#L2742

https://github.com/gyurco/Arcade-IremM92_MiSTer/blob/700bcbad15e3caace61580b50b394284896b499b/rtl/v30/cpu.vhd#L2744

sorgelig commented 1 year ago

Left = current core right = gyurco's changes

From your pics i don't see noticeable optimization in resources. Probably something went wrong.

birdybro commented 1 year ago

See total registers.

ALMS: 29054 --> 28839 Total Registers: 31787 --> 29759

And the Cyclone III in the MiST board has different arrangements for how they define ALMs, so this might have been a bigger difference.

BRAM was increased slightly as a result of the inferred decryption table.

wickerwaka commented 1 year ago

Thanks for porting this, @gyurco, I'm happy that MiST users will get to play it. I thought of MiST a few times during development and tried to keep BRAM usage as low as I could.

I'm not sure yet whether I want to merge the v30 and v35 core versions so I'm going to leave this PR open for now while I work on some other changes.

gyurco commented 1 year ago

Total Registers: 31787 --> 29759 2k less registers, it's almost directly translatable to 2k LCs on the MiST's FPGA. Looks like it's less visible in terms of ALMs. BTW, with Quartus 17.1, it synthesises to slightly less ALMs, too (28545 instead of 28839).

gyurco commented 1 year ago

Thanks for porting this, @gyurco, I'm happy that MiST users will get to play it. I thought of MiST a few times during development and tried to keep BRAM usage as low as I could.

At the end, some more stuff had to be outsourced to external RAM, so 8 different subsytem ended up in SDRAM. I think it's really maxed out :)

birdybro commented 1 year ago

It's awesome that you were able to get it to fit. MiST has stuck around for awhile!!