Integrate combin memory into pipelined and single-cycle CPUs #82

jardhu · 2019-07-24T10:47:51Z

This PR (hopefully) modifies the existing CPU models to use the newer memory with no side effects to overall functionality.

jardhu · 2019-07-24T10:54:54Z

At the moment the code does not work because I've hit a hard wall with that one treadle simulator issue. I've tried to remove all relevant DontCares from the memory modules, but the NoSuchElementException still gets raised with the DualPortedCombinMemory.memory module. The elaborated firrtl doesn't have any invalid signals anywhere in the three memory modules (DualPortedCombinMemory, ICombinMemPort, and DCombinMemPort).

It doesn't work with the non-combinational memory, either - the same exception is raised.

If DCE really is the case of this problem, I may experiment with adding the firrtl.transforms.NoDCEAnnotation option to the simulator's optionsManager as suggested by chipsalliance/chisel#971 though I am not sure if that would be a desired long-term fix.

powerjg · 2019-07-24T18:49:21Z

That seems like a fine short term fix. Does it work if you add that option?

powerjg

LGTM. I assume that all of the tests are using the combinational memory, correct?

src/main/scala/components/coreio.scala

jardhu · 2019-07-25T07:15:51Z

Finally had some time to work on splitting the IO. That flipped bus definitely seemed to be the issue, since the CPU is now sending signals to the memory and the memory address size check assert is failing.

powerjg · 2019-07-25T17:25:20Z

Changes look good! Sounds like we're getting somewhere!

jardhu · 2019-07-26T10:55:31Z

Alright, I got almost all of the single cycle CPU tests passing. The ones that do fail are due to the CSR evec signal being DontCare/invalid, so the imem address gets assigned to 0x80000000. This address is too large for the memory address assert.

I can work around this by removing that assert and outputting 0 when the address exceeds the memory's size, which is what the old memory does, but ideally we should have that assert when the CSR is fully incorporated into the CPU.

powerjg · 2019-07-26T15:06:58Z

This looks good to me. However, I would prefer if you could rebase and clean up the history to make sure that each commit is its own thing.

As a couple of examples:

make sure to keep the csr thing in its own commit
Make changes to the CPUs their own commits
Moving CoreIO around
Etc.

jardhu · 2019-07-27T03:33:48Z

I rebased most of the combinational memory work. Hopefully the commit history is more clear.

My plan right now is to integrate the non-combinational memory into the pipelined CPUs, then rebase any frivolous commits starting at b44429b if needed before the final merge.

powerjg · 2019-07-31T00:53:49Z

Looks like we're ready to merge??? :D

jardhu · 2019-07-31T03:40:36Z

Not yet, the non combin memory does not work with the pipelined CPUs, but the combinational memory does. I'm debugging them right now, seems to be how I'm stalling the CPU.

@powerjg The best way to stall the entire CPU would be to set the if_id bubble to true, ex_mem bubble to true, and pcwrite to 2.0 so that the CPU doesn't proceed, correct?

powerjg · 2019-07-31T15:14:01Z

Seems reasonable, but let's talk more details in our meeting today.

…

On Wed, Jul 31, 2019 at 4:01 AM Jared Barocsi ***@***.***> wrote: So, I need to make a more fundamental change to the design of the pipelined CPU that will deviate slightly from the pattern in Patterson & Hennessy. I need to be able to freeze the ex_mem register - right now, the CPU flushes the register with 0 when there is a bubble <https://github.com/jlpteaching/dinocpu/blob/bb0ca05b56d5dd7503555851c1abbbd1d266ad70/src/main/scala/pipelined/cpu.scala#L296-L302>, but the control signals otherwise propagate through to the mem_wb stage. This results in some *desync* between the writeback signals and the readdata signal, which is delayed. I took the lw1 test and added some relevant debug info: ... >>> Cycle #7 PC: 12 1 => 0 => 0 (readdata = 0) Issuing a read >>> Cycle #8 PC: 16 0 => 1 => 0 (readdata = 0) *** Stalling! *** >>> Cycle #9 PC: 20 0 => 0 => 1 (readdata = 0) *** Stalling! *** >>> Cycle #10 PC: 20 0 => 0 => 0 (readdata = 0) *** Stalling! *** >>> Cycle #11 PC: 20 0 => 0 => 0 (readdata = 0) *** Stalling! *** Data received: 4294967295 >>> Cycle #12 PC: 20 0 => 0 => 0 (readdata = 0) *** Stalling! *** >>> Cycle #13 PC: 20 0 => 0 => 0 (readdata = 0) >>> Cycle #14 PC: 24 0 => 0 => 0 (readdata = 4294967295) >>> Cycle #15 PC: 28 0 => 0 => 0 (readdata = 0) This corresponds with a lw t0, 0x400(zero) instruction entering the mem stage, being delayed for five cycles (the configured latency), and continuing on with execution. The third line of each cycle printout (x => x => x) corresponds with the wbcontrol.toreg signal in the id_ex, ex_mem, and mem_wb registers respectively. The readdata corresponds with mem_wb.readdata. The main issue is that at cycle #9 <#9> (a stalling cycle), the desired toreg signal, 1, is located in mem_wb, so the CPU will write back the contents of readdata, which is 0. At cycle #14 <#14>, mem_wb.readdata is set to 0xFFFFFFFF, but toreg is 0 in mem_wb. So the readdata is lost entirely. Ideally I must be able to freeze ex_mem as so: ... >>> Cycle #7 PC: 12 1 => 0 => 0 (readdata = 0) Issuing a read >>> Cycle #8 PC: 16 0 => 1 => 0 (readdata = 0) *** Stalling! *** >>> Cycle #9 PC: 20 0 => 1 => 0 (readdata = 0) *** Stalling! *** >>> Cycle #10 PC: 20 0 => 1 => 0 (readdata = 0) *** Stalling! *** >>> Cycle #11 PC: 20 0 => 1 => 0 (readdata = 0) *** Stalling! *** Data received: 4294967295 >>> Cycle #12 PC: 20 *** Stalling! *** 0 => 1 => 0 (readdata = 0) >>> Cycle #13 PC: 20 0 => 1 => 0 (readdata = 0) >>> Cycle #14 PC: 24 0 => 0 => 1 (readdata = 4294967295) >>> Cycle #15 PC: 28 0 => 0 => 0 (readdata = 0) >>> Cycle #16 Which would synchronize the readdata and toreg signals properly. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#82?email_source=notifications&email_token=AAA2YHHY2VF6BQSB5QVINRTQCFWHBA5CNFSM4IGOW5K2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3G4LVQ#issuecomment-516802006>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAA2YHG3RYMGGLWBC5FVY33QCFWHBANCNFSM4IGOW5KQ> .

-- Jason Lowe-Power (he/him/his) Assistant Professor, Computer Science Department University of California, Davis 3049 Kemper Hall https://arch.cs.ucdavis.edu/

jardhu · 2019-08-02T00:46:22Z

Commit history rebased to stash away code for the non-combin memory, the PR should only involve the combinational memory except for the pipeline/bus IO commit.

I'm getting some strange dependency cycle/StackOverflowError that seems to happen randomly, but I'm positive all of the tests should pass after this force push.

jardhu · 2019-08-02T00:58:03Z

@powerjg, seems like the Chisel snapshots screwed up and now there's a dependency desync. Could you perhaps look at it? The only thing preventing this PR from being merged is a compilation error in the replrunner, which I haven't touched at all.

powerjg · 2019-08-02T15:21:34Z

There are actually two issues. First, treadle was updated. I made the required changes to the replrunner to compile. Then, there's an issue with the output directory. I'm seeing the following:

[error] (run-main-1) firrtl.options.OptionsException: Exactly one target directory must be specified, but found `test_run_dir/pipelined/add1, test_run_dir/pipelined/add1` specified via:
[error]     - explicit target directory: -td, --target-dir, TargetDirAnnotation
[error]     - fallback default value
[error] firrtl.options.OptionsException: Exactly one target directory must be specified, but found `test_run_dir/pipelined/add1, test_run_dir/pipelined/add1` specified via:
[error]     - explicit target directory: -td, --target-dir, TargetDirAnnotation
[error]     - fallback default value

I found this: https://groups.google.com/forum/#!topic/chisel-users/SgB7M65F4Ow

If you comment out line 19 of CPUTesterDriver.scala things seem to work, but it makes a mess in the CWD.

PS: There's a bunch of warnings in configuration.scala. I'm not sure what they mean.

powerjg · 2019-08-08T15:28:09Z

Looks great to me! Please rebase then merge!

PS: Could you open an issue about the output directory for the tester?

All single cycle CPU tests now pass.

Adds parallel documentation to DMemPortIO and fixes some whitespace. Signed-off-by: Jason Lowe-Power <[email protected]>

Signed-off-by: Jason Lowe-Power <[email protected]>

This will pollute the CWD with testing files in the meantime. Be sure to not `git add .`, and instead add the files you actually changed.

powerjg approved these changes Jul 24, 2019

View reviewed changes

powerjg reviewed Jul 24, 2019

View reviewed changes

src/main/scala/components/coreio.scala Show resolved Hide resolved

jardhu force-pushed the integrate-new-memory branch from 49e4b66 to 9223254 Compare July 25, 2019 21:00

jardhu force-pushed the integrate-new-memory branch from e0650dc to c7f6ec7 Compare July 27, 2019 03:30

jardhu force-pushed the integrate-new-memory branch 3 times, most recently from 581efaf to f4827be Compare July 27, 2019 07:00

jardhu changed the title ~~Integrate combin/noncombin memory~~ Integrate combin memory into pipelined and single-cycle CPUs Aug 2, 2019

jardhu force-pushed the integrate-new-memory branch from b88c78f to 388baa0 Compare August 2, 2019 00:45

Jared Barocsi added 7 commits August 8, 2019 08:39

Move CPU IO into an abstract BaseCPU module

bf85eaa

Modify top and config to use new memory modules

330f371

Set base branchpred and mem modules as abstract

4d5e7e9

Remove deprecated memory

562f2d6

Separate port IO into 'pipeline' and 'bus' bundles

8614938

Workaround for invalid/DontCare address from CSR

c6af3c7

All single cycle CPU tests now pass.

Integrate combin memory into single-cycle CPU

16b32eb

Jared Barocsi and others added 9 commits August 8, 2019 08:40

Integrate combin memory into pipelined CPU

a5c8d04

Integrate combin memory into pipelined/bp CPU

e65ca36

Minor updates to MemPortIO

dc82cea

Adds parallel documentation to DMemPortIO and fixes some whitespace. Signed-off-by: Jason Lowe-Power <[email protected]>

Write up memory documentation

31f5481

Copy MemPortIO code doc into documentation

8100715

Add missing port<=>memory comment

d217f20

Fix combin memory test to observe correct good

af14f7d

Update replrunner with new changes to Treadle

9b11b5f

Signed-off-by: Jason Lowe-Power <[email protected]>

Temporarily fix tester driver CWD issue

429192b

This will pollute the CWD with testing files in the meantime. Be sure to not `git add .`, and instead add the files you actually changed.

jardhu force-pushed the integrate-new-memory branch from 10ecfbd to 429192b Compare August 8, 2019 15:44

jardhu merged commit 9ffea16 into jlpteaching:master Aug 8, 2019

jardhu mentioned this pull request Aug 8, 2019

Fix Treadle output directory bug #84

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate combin memory into pipelined and single-cycle CPUs #82

Integrate combin memory into pipelined and single-cycle CPUs #82

jardhu commented Jul 24, 2019

jardhu commented Jul 24, 2019 •

edited

Loading

powerjg commented Jul 24, 2019

powerjg left a comment

jardhu commented Jul 25, 2019 •

edited

Loading

powerjg commented Jul 25, 2019

jardhu commented Jul 26, 2019

powerjg commented Jul 26, 2019

jardhu commented Jul 27, 2019 •

edited

Loading

powerjg commented Jul 31, 2019

jardhu commented Jul 31, 2019 •

edited

Loading

powerjg commented Jul 31, 2019 via email

jardhu commented Aug 2, 2019

jardhu commented Aug 2, 2019 •

edited

Loading

powerjg commented Aug 2, 2019

powerjg commented Aug 8, 2019

Integrate combin memory into pipelined and single-cycle CPUs #82

Integrate combin memory into pipelined and single-cycle CPUs #82

Conversation

jardhu commented Jul 24, 2019

jardhu commented Jul 24, 2019 • edited Loading

powerjg commented Jul 24, 2019

powerjg left a comment

Choose a reason for hiding this comment

jardhu commented Jul 25, 2019 • edited Loading

powerjg commented Jul 25, 2019

jardhu commented Jul 26, 2019

powerjg commented Jul 26, 2019

jardhu commented Jul 27, 2019 • edited Loading

powerjg commented Jul 31, 2019

jardhu commented Jul 31, 2019 • edited Loading

powerjg commented Jul 31, 2019 via email

jardhu commented Aug 2, 2019

jardhu commented Aug 2, 2019 • edited Loading

powerjg commented Aug 2, 2019

powerjg commented Aug 8, 2019

jardhu commented Jul 24, 2019 •

edited

Loading

jardhu commented Jul 25, 2019 •

edited

Loading

jardhu commented Jul 27, 2019 •

edited

Loading

jardhu commented Jul 31, 2019 •

edited

Loading

jardhu commented Aug 2, 2019 •

edited

Loading