google / google/xls

DSLX: Rust-style attributes

Open
#687 0 comments 0 reactions 0 assignees View on GitHub
dslx enhancement
Dominant language
C++
Stars
1.9k
Forks
283
Avg merge
2d 10h
Merged PRs (30d)
135

Description

Right now, we have `test`, `test_proc`, `cfg`, and `quickcheck` "directives" which are similar to [rust attributes](https://doc.rust-lang.org/reference/attributes.html)[^1]. These directives result in different ast nodes being parsed, e.g. `#[test] fn some_test_function() { ... }` does not parse to a `Function` with a `test` attribute, it parses to a `TestFunction`. In playing around with getting SRAM support working, it seems to me that a version of the latter would be helpful, for example:

```
#[IOConstraint("req_chan:send", "resp_chan:recv", min_latency=1, max_latency=1)]
proc Sram {
#[ChannelConfig(flow_control="valid_only")]
req_chan: chan in SramReq;

#[ChannelConfig(flow_control="valid_only")]
resp_chan: chan out SramResp;

config(...) { ... }
next(...) { ... }
}
```

I've played around a bit with what it would look like to add this to the ast+parser, and here are some questions I have:

1. I'm being pretty loose w/ what's allowed in the argument list. It would be nice to have named arguments. I could imagine the IO constraint being inside the body and referencing the channels directly, like `#![IOContraint(req_chan, kSend, resp_chan, kRecv, min_latency=1, max_latency=1)]`.
2. What entities should accept attributes? This seems to be [a matter of some debate for rust](https://rust-lang.github.io/rfcs/0016-more-attributes.html) too. If enough entities have attributes, perhaps an attribute list should be a member of `AstNode`.
3. Related to 2., should the parser do
```
list_of_attributes = TryParseAttributes()
if peek_token() == OpA:
return ParseOpA(list_of_attributes)
elif peek_token() == OpB:
return ParseOpB(list_of_attributes)
...
```
or
```
def ParseOpA():
list_of_attributes = TryParseAttributes()
```
The first is needed if you're dispatching on a token (you need to get through the attributes to know what to dispatch on). The second is more natural if you're not dispatching on a token (it encapsulates the op better). It would be nice to be consistent and make the parsing steps more composable.

I suppose you could also do
```
next_op_token = peek_token_after_attributes()
if next_op_token == OpA:
return ParseOpA()
if next_op_token == OpB:
return ParseOpB()
```
or similar using transactions and have `ParseOpA()` and `ParseOpB()` actually parse the attributes. Is scanning forwards and backwards too wasteful?
4. It seems useful to go beyond built-in attribute macros. You could imagine something writing a macro implementation in C++/Python/whatever, and using a DSLX import (or `use` statement?) to pull in the macro implementation.

[^1]: As a side-node, it seems that we're incorrectly using inner attributes instead of outer attributes for most of these.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.