llvm / llvm/circt

[Comb] Clarification on non-two-state operations

Open
#4,428 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
2.2k
Forks
524
Avg merge
3d 2h
Merged PRs (30d)
46

Description

#3766 introduced the `twoState` attribute to many operators in comb, but I am struggling to get the intended consequences of this flag not being set. The way I understand it, any transformation on operations without this flag should preserve the behavior for all (System)Verilog 4-state and VHDL 9-state values. However, apart form being highly restrictive, I do not think this is what the current implementation does. For example, the SV spec states:

> For the arithmetic operators, if any operand bit value is the unknown value x or the high-impedance value z,
then the entire result value shall be x.

Thus, even trivial optimizations like `mul(a, 0) -> 0` would have to be disabled for any operation without the `twoState` attribute. Yet,

```
hw.module @zeromult(%a: i8) -> (y: i8) {
%c0_i8 = hw.constant 0 : i8
%0 = comb.mul %a, %c0_i8 : i8
hw.output %0 : i8
}
```

is canonicalized to

```
hw.module @zeromult(%a: i8) -> (y: i8) {
%c0_i8 = hw.constant 0 : i8
hw.output %c0_i8 : i8
}
```
even without the `bin` flag. So, effectively the behavior seems to be undefined for non-two-state values at the moment or am I missing something? Personally, I think there needs to be a reasonable middle-ground between strictly preserving the behavior and completely ignoring the existence of non-two-state values, but we would have to specify it.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Begin with #3766 and the comb canonicalization of the shown mul(a, 0) example. Compare the intended twoState behavior with SystemVerilog four-state and VHDL nine-state semantics, then document or implement a decided middle ground and verify that the example follows it.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.