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

const variables #65

Closed
suarezvictor opened this issue Mar 20, 2022 · 12 comments
Closed

const variables #65

suarezvictor opened this issue Mar 20, 2022 · 12 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@suarezvictor
Copy link

for variables declared as const, never generate registers for the pipeline (just wires)
This will ease the job of the synth tool and be easier than tracking if the variables changes (and makes syntax and purpose of functions more clear)

@JulianKemmerer
Copy link
Owner

A large part of this is figuring out how to make the place and route tools not fail on these 'const' paths once they are there... Need to structure code as to be able to use PNR specific things settings for turning off timing on certain paths

False paths , multi paths, async domains ... something

@JulianKemmerer
Copy link
Owner

Also const is a good keyword idea

But I want to make sure it's not confuse:
this values is slowly changing and appears constant vs
This value is actually a constant (that propagates and optimizes logic away)

Maybe getting at const vs constexpr ?

@JulianKemmerer
Copy link
Owner

JulianKemmerer commented Mar 20, 2022

Can you use const variables to calculate other const variables?
Do we have to have const. functions now?
I suppose FUNC WIRES pragma is one way to signal a func only does constant wiring etc - so maybe can use WIRES marked funcs for any calculations using const vars... Idk it's not the same situation exactly

@JulianKemmerer JulianKemmerer self-assigned this Mar 20, 2022
@JulianKemmerer JulianKemmerer added the enhancement New feature or request label Mar 20, 2022
@JulianKemmerer
Copy link
Owner

nextpnr info on constraining clocks (and lack of clocks?)
https://github.com/YosysHQ/nextpnr/blob/master/docs/constraints.md#clock-constraints

@JulianKemmerer
Copy link
Owner

JulianKemmerer commented Mar 20, 2022

Looks like nextpnr can't false path certain logic

So will probably need to have the slow per frame logic as 'a slow other clock domain' and the default behavior will be to not time the comb paths between

@JulianKemmerer
Copy link
Owner

I realize now that I have been confusing the simple request for "const meaning don't generate registers" with a more complex set of functionality that is specific to our project Victor

I plan to create issues that break down how to use const as the last step in more advanced functionality and not make this issue more complicated than it needs to be @suarezvictor

@JulianKemmerer
Copy link
Owner

JulianKemmerer commented Mar 21, 2022

Ok outlining what needs to happen in order to implement this issue: @suarezvictor

"Const means no regs for that variable". A consequence of removing registers along variable/wires means now previously separate chunks of combinatorial logic are no longer separate and can form long delay paths. The PipelineC tool wont be able to pipeline these regions if they become part of the design critical path. The tool will likely get stuck iterating forever or end failing to meet timing in that case. Adding 'const' shouldn't ever make your design not meet timing/make pnr fail. :-/ The user can work around this issue by adding their own registers around the 'const' usage to fix the paths PipelineC can't, but it's not something to assume of people.
To make this work PipelineC needs to communicate to the place and route tool one of two things to make const always work for the user:

  1. false paths: that certain paths in the design are part of this 'const' area of no registers and to never time combinatorial logic paths there. It needs to create what is called 'false paths'.This introduces two big challenges: 1) This is specific to every PNR tool. And some do not support them (ex. nextpnr does not support false paths). So just knowing if/how each tool lets you do this is one task. 2) To go from 'const' variables names in C to the inputs of whatever tool specific set false paths commands are needed. Ex. some tools need paths between registers (not just paths along comb. logic/LUTs) - well what if the marked 'const' variable isnt a 'static'/register as well? Likely need to traverse the netlist tracing out where this 'const' marked variable propagates to. Does 'const' propagate across assignments/function calls? 
    Implement false path support  #66

  2. the path is timed but very very slow clock: Introduce another clock domain slow enough to meet your "so slow its constant '' rate. Clocks that slow are typically produced from user counters in FPGA fabric - which is not yet supported. Right now users can 'fake' this by manually returning/outputing their user clock signal like it was going 'off chip' but then outside of the design top level looping back connecting the output clock back like another other 'normal clock'. Its an open issue to automate this user 'faking loopback'
    https://github.com/JulianKemmerer/PipelineC/issues/67 
    Also the concept of a clock domain crossing where one side is "so slow its ~constant" is not really supported. You would want to treat this like a wire - no buffering/fifoing like typical clock crossing. More like an async wire 'always just a wire clock crossing' which is not yet implemented.Add 'always a wire' clock crossings #68

Finally - I think 'const' variables in general makes sense for 1) false paths. But I dont think const is the best way to communicate 2) "the path is timed but very very slow clock" because paths between clock domains is something pipelinec already has concepts for. In the issues listed under 2) the missing support can be added and I think we can get close to what 'const' variables is trying to do without fully implementing false paths which sadly nextpnr doesnt support right now.

@suarezvictor
Copy link
Author

I understand the complexity and thank you take the time to address it. However I doubt it's only useful for graphics projects. I think this would apply to any kind of computations using nested loops. Think of iterative matrix computation for example. While you calculate let's say columns, rows are fixed. I think many concrete examples will appear where some data changes at high speed while other data remains constant till the end of the fast computation.

@JulianKemmerer
Copy link
Owner

I think its worth it to note @suarezvictor

I said Adding 'const' shouldn't ever make your design not meet timing/make pnr fail. As part of my concerns above, from a usability, ex. 'how is this likely to be used by some software person' perspective.

With that said, I am open to something intentionally less friendly perhaps something like a volatile local variable. Some other keyword signalling 'this is weird and watch out what you do with it' (as opposed to 'safe and familiar const' feeling which is misleading without alot of back end tool support - see my earlier comments on this issue).

There are already volatile static registers.
https://github.com/JulianKemmerer/PipelineC/wiki/Volatile-Global-Variables

So maybe some thought into if that is confusing?

@JulianKemmerer
Copy link
Owner

Also if we can't settle on a keyword can always try a pragma ☺️

@JulianKemmerer
Copy link
Owner

Per #68 there is a #pragma ASYNC_WIRE my_wire that exists now to mark global wires as async. A similar thing could be applied to local variable + input args as well.

The real work is then propogating this ASYNC (or volatile or whatever its called) across logic. As I said earlier Likely need to traverse the netlist tracing out where this 'const'/ASYNC marked variable propagates to. Does 'const'/ASYNC propagate across assignments/function calls?

@JulianKemmerer
Copy link
Owner

As of ccd0e81
the original goal of never generate registers for the pipeline (just wires) was completed.

As seen in the many comments - this did spawn several related issues of future work still.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants