xls icon indicating copy to clipboard operation
xls copied to clipboard

Support channel legalization strictness parameter in frontends

Open grebe opened this issue 2 years ago • 6 comments

An upcoming change will move channel legalization strictness from being a global option to a per-channel option. For example, one channel could be configured as requiring mutually exclusive usages while another could be configured as allowing multiple accesses per tick.

Currently, the DSLX and XLSCC frontends do not set this option in the IR, so the default 'proven mutually exclusive' option will be used everywhere. Frontends should support ways of configuring channels in the IR they produce.

grebe avatar Jun 12 '23 20:06 grebe

@grebe Can you propose what this should look like in the DSL?

cdleary avatar Aug 08 '23 23:08 cdleary

In DSLX, I'd propose an optional keyword "template parameter" for chan<T>.

I think we could go with chan<T, strictness=total_order>, etc.

ericastor avatar May 20 '24 14:05 ericastor

On more consideration, I'd propose the following syntax, using an attribute on chan members of a proc:

#[strictness(total_order)]
ch: chan<T> in;

or possibly: ch: chan<T> in #![strictness(total_order)];, our first inner attribute. The exact attribute name is subject to appropriate amounts of bikeshedding.

  • Pros:
    • No new language features required
    • Relatively concise
  • Cons:
    • Makes an attribute load-bearing for code correctness & legality
      • This can be somewhat mitigated if we make the default an option that allows reuse at a PPA penalty, so users have to opt into the low-overhead restricted-use proven_mutually_exclusive option.

Alternatives considered:

  1. Optional keyword template parameter for chan:
    • ch: chan<T, strictness=total_order> in;
    • Pros:
      • well-contained, syntactically concise
    • Cons:
      • introduces optional parameters to DSLX
      • appears to apply to the channel type, while really only being about the local interface
  2. New keywords trailing a channel member
    • ch: chan<T> in reusable_in_total_order; (keyword deliberately chosen as a bad placeholder)
    • Pros:
      • Concise syntax
    • Cons:
      • Adds multiple new keywords
      • No obvious naming choice for clarity in the new keywords (in my opinion)

ericastor avatar Jul 18 '24 16:07 ericastor

One issue with the above proposal is where to bind the strictness:

We have channels arise in the following ways

  • A chandecl, i.e. let (...) = chan<u32>("name");
  • A top-level proc's config arguments, i.e. config(ch:chan<u32>in) { .. }
  • A spawned proc's config arguments, which don't create a new channel

Any one of these channels can be bound to a proc's member channels, which in the above proposal can be annotated with a strictness. As a result, the same channel can be annotated multiple times, potentially with different strictness. We should be thoughtful about what is allowed and how this works.

Another thing to note is that (currently) strictness in the IR applies to the whole channel, but you could imagine different strictness in different procs. Perhaps it should be an annotation on channel references. We currently require different procs to have mutually exclusive usage of the channels and tick in lockstep, but this possibly more restrictive than we want.

grebe avatar Jul 18 '24 19:07 grebe

Related: #687 and #1561.

An idea I've been playing around with is the following:

#[strictness(runtime_mutually_exclusive)]
let (...) = chan<u32>("name");

This is similar to @ericastor's earlier suggestion, except instead of applying to the channel member, the attribute is applied to a chandecl statement.

Pros:

  • The chandecl is where both ends of the channel are made so you don't need to resolve what happens when different procs mark the same channel (possibly different ends of it) with different strictness.

Cons:

  • This doesn't address top-level channels which are made from config args rather than from a chandecl.
  • You need to decide what you do with things like let (a, b) = #[strictness(runtime_mutually_exclusive)] { let (c, d) = chan<u32>("name"); (d, c) } or worse. Probably it's OK to require the strictness attribute to bind closely to the chandecl.
  • Attributes probably shouldn't be able to take a parameterized value, i.e. #[strictness(PROC_PARAM_STRICTNESS)]. I think this is more of a concern for #1561, but maybe you also want strictness to be parameterized?

grebe avatar Sep 12 '24 00:09 grebe

Perhaps the suggestion in #1561 to have different chandecl syntaxes is the way to go here too. I could imagine something like:

let (...) = runtime_mutually_exclusive::chan<u32>("name"); or let (...) = fifo::runtime_mutually_exclusive<u32>("name");.

grebe avatar Sep 12 '24 00:09 grebe