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

[ExportVerilog] Add support for UnionCreate op. #5081

Merged
merged 1 commit into from
Apr 25, 2023

Conversation

youngar
Copy link
Member

@youngar youngar commented Apr 25, 2023

This adds support for hw::UnionCreateOp to ExportVerilog. It works by emitting a bit concatenation and assigning it to a wire with a union type. Each union member may have a prepadding, which is decided by the member's offset size, and a postpadding to ensure that each union element is the same size. The resulting bitconcat is {prepadding, data, postpadding}. If there is no padding for a particular member, then bitconcat will not be used.

The following example has no offsets, but the member a requires post-padding of 1 bit.

hw.module @unionCreate(%in: i1) -> (out: !hw.union<a: i1, b: i2>) {
    %0 = hw.union_create "a", %in : !hw.union<a: i1, b: i2>
    hw.output %0 : !hw.union<a: i1, b: i2>
}
module unionCreate(
  input                                                                                          in,
  output union packed { struct packed {logic a; logic [0:0] __post_padding_a;} a;logic [1:0] b;} out
);

  assign out = {in, 1'h0};
endmodule

This adds support for `hw::UnionCreateOp` to ExportVerilog.  It works by
emitting a bit concatenation and assigning it to a wire with a union
type.  Each union member may have a prepadding, which is decided by the
member's offset size, and a postpadding to ensure that each union
element is the same size. The resulting bitconcat is `{prepadding, data,
postpadding}`.  If there is no padding for a particular member, then
bitconcat will not be used.

The following example has no offsets, but the member `a` requires
post-padding of `1` bit.

```mlir
hw.module @unionCreate(%in: i1) -> (out: !hw.union<a: i1, b: i2>) {
    %0 = hw.union_create "a", %in : !hw.union<a: i1, b: i2>
    hw.output %0 : !hw.union<a: i1, b: i2>
}
```

```verilog
module unionCreate(
  input                                                                                          in,
  output union packed { struct packed {logic a; logic [0:0] __post_padding_a;} a;logic [1:0] b;} out
);

  assign out = {in, 1'h0};
endmodule
```
@youngar youngar marked this pull request as ready for review April 25, 2023 19:36

// If the element has no padding, emit it directly.
if (elementWidth == unionWidth) {
emitSubExpr(op.getInput(), Selection);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uenoku I'm not sure if this is the correct precedence here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UnionCreate seems to be spilled to a wire so there is no correctness issue but I think LowestPrecedence is better. There is an extra parentheses for the following example:

hw.module @unionCreate(%in: i2) -> (out: !hw.union<a: i2, b: i2>) {
    %add = comb.add %in, %in : i2
    %0 = hw.union_create "a", %add : !hw.union<a: i2, b: i2>
    hw.output %0 : !hw.union<a: i2, b: i2>
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed here: #5091
Thanks Hideto!

@youngar youngar merged commit a4b8f96 into llvm:main Apr 25, 2023
@youngar youngar deleted the exportverilog-createunion branch April 25, 2023 20:38
@teqdruid
Copy link
Contributor

Sorry about the noise -- was looking at an old rev.

@youngar
Copy link
Member Author

youngar commented Apr 25, 2023

Thanks for taking a look @teqdruid, I would be grateful for any post-merge suggestions

Copy link
Contributor

@teqdruid teqdruid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, though I'm also not sure about the precedence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants