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

Adding CFGLUT5 primitive #249

Closed
wants to merge 13 commits into from
Closed

Conversation

vcanumalla
Copy link
Collaborator

@vcanumalla vcanumalla commented May 15, 2023

This PR aims to add support for the CFGLUT5 primitive.

Closes #259

See: https://github.com/uwsampl/lakeroad-private/pull/1

@gussmith23
Copy link
Owner

I don't think we can have IS_CLK_INVERTED be a port; when it is a port, we need to change the Verilog such that the register is set on both the positive edge and the negative edge of the clock, which Yosys doesn't like.

@gussmith23
Copy link
Owner

@vcanumalla here's a Verilog hack you can use, to attempt to get INIT working the way we want it to.

First, what does INIT do? It controls the initial value in the memory. You can see that from this line:

  reg  [31:0] data = INIT;

And also these lines:

  initial
  begin
    assign  data = INIT;
  ...

I'm pretty sure these lines are totally redundant, but maybe they're not. Both of these lines set data to INIT as soon as the module is "started" (which is when initial blocks are "executed". Again, remember, this is hardware, so everything exists all the time and is never "run" per se).

The current way INIT is used isn't usable for us, b/c we need to convert it to a port, and then we'll get Yosys errors, because you can't use a port to initialize a value in an initial block (because what would that even mean?)

So here's idea 1: Can you write some Verilog code that will set data to the value of the INIT port whenever INIT changes? This should be pretty short.

I suspect idea 1 will give us Yosys errors. It will probably complain that data has multiple drivers, or something like that. If it does, we'll improve the solution.

Also, if we wanted to make idea 1 more correct, we'd probably add a flag that would make it so that data is updated from INIT once and only once -- you can give this a try as well. Interestingly, there's already some code in there that does something similar...I'm not sure why it's there, though.

@gussmith23
Copy link
Owner

gussmith23 commented Nov 10, 2023

Heads up @vcanumalla: I'd really love to get this to a place where we can merge it, even if it's partially complete. Minimal amount I think we'd need to do to get there:

  • Write documentation for the new things that were added (at the very least, for the sketch)
  • Make at least one non-trivial test that works
  • Make sure we get the CFGLUT5 Verilog merged in to lakeroad-private, and make sure we update the submodule pointer in lakeroad.
  • Resolve merge conflicts with main

@gussmith23 gussmith23 mentioned this pull request Nov 10, 2023
@gussmith23 gussmith23 closed this Sep 18, 2024
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.

Support Xilinx CFGLUT5 primitive
3 participants