Single Port Memories

[andrewb1999]

This is definitely still a work in progress, mostly need to fix all the frontend test cases. Will need to coordinate with all the frontends too.

[rachitnigam]

@EclecticGriffin is this a hard change to make in the interpreter?

[EclecticGriffin]

This shouldn't be too hard to implement in the interpreter, the only weirdness being that some adjustments will be needed in the SeqMem state-machine since there's no longer an explicit error state if I'm understanding things correctly. I can handle that today probably if desired

[EclecticGriffin]

Okay took care of the interpreter side, though I can't be sure there aren't any obvious mistakes until whatever is upsetting the parser is fixed

[andrewb1999]

Not exactly sure why the parser is failing here (and I don't have enough experience with the parser to debug this). Any ideas @rachitnigam ?

[EclecticGriffin]

my guess is that it has to do with the static annotation? I'm not clear on whether or not the old style is fully deprecated yet, but the parser does treat static as a keyword and so might be mis-parsing because of that? Here's a stupid idea, try reordering the annotations so that the static attribute is never the first one. I think the repeated attribute parsing might not explode then?

[rachitnigam]

Weird error!

[rachitnigam]

Ah, missing comma after content_en is the problem! Normally, if you forget commas, error locations become really inscrutible!

[rachitnigam]

Should be fixed now!

[rachitnigam]

Remaining failures seem legitimate. We also need to update Dahlia to use the right ports. Maybe we can recruit @calebmkim's help on this

[calebmkim]

Yeah I can try to implement a fix for Dahia... should be pretty simple.

[calebmkim]

One thing I'm noticing is that we should change the @write_together annotations. Right now write_data and write_en are required to be driven together, but we shouldn't necessarily require that, since if we want to read, we would drive write_en to 0, and not set write_data.

I think we probably want the following ports driven together: content_en, write_en, and all the address ports. Any time you set content_en to high, you should specify a) whether to read/write and b) which memory addresses to access.

[rachitnigam]

This is getting stale. @andrewb1999 what are the specific blockers to getting this merged?

[andrewb1999]

I was mostly just waiting for front end changes to make sure this won't break stuff, but yeah this is getting stale now. I'm happy to fix the staleness of it but before I do that we should make sure we are ready to merge and fix any issues that may arise with frontends.

[rachitnigam]

@andrewb1999 sorry for the slow turnaround on this! Let's start the process of merging it. We can handle the Dahlia side of things. I think getting this done will also unblock #1261 which we should do soon.

[rachitnigam]

Pushed fixes to this branch! Should be ready to merge once the tests pass.

[rachitnigam]

Blocked by #1919

[rachitnigam]

Ready to merge now!

[rachitnigam]

Ugh, we'll need to update the outputs from the relay test suite but I don't have that working on my machine...