Resolve combinational loops

[tancheng]

The val/rdy ifcs are leveraged for hand-shake.

• We allow rdy to be dependent on val, but should avoid val depending on rdy, otherwise, loops exist.

Related issues:

• [P0] Synthesis-friendly tile design #13
• [P2] Cleanup and update #39

[tancheng]

@yuqisun python 3.7 doesn't exist on latest ubuntu (actions/runner-images#10636), seems like the new rule is starting being applied in our test workflow.

Do you think we need to keep ubuntu be 22.04, or deprecate python 3.7?

[yuqisun]

@yuqisun python 3.7 doesn't exist on latest ubuntu (actions/runner-images#10636), seems like the new rule is starting being applied in our test workflow.

Do you think we need to keep ubuntu be 22.04, or deprecate python 3.7?

May keep ubuntu-20.04 (same as CGRA-Mapper) until get LLVM upgraded?
Tried in OS, not easy to install different version LLVM with default in OS, not sure how about in github workflow, can try later. If all good, we can deprecate py37, WDYT?

2 options to upgrade ubuntu:

1. Install LLVM-12 in new ubuntu (not easy in OS, you may try with mac also)
2. Upgrade LLVM version in code (sounds better?) (but remember there's arguments change between LLVM 12 and 18)

For now, we can use ubuntu-20.04 or disable py37 to pass build right?

[tancheng]

@yuqisun python 3.7 doesn't exist on latest ubuntu (actions/runner-images#10636), seems like the new rule is starting being applied in our test workflow.
Do you think we need to keep ubuntu be 22.04, or deprecate python 3.7?

May keep ubuntu-20.04 (same as CGRA-Mapper) until get LLVM upgraded? Tried in OS, not easy to install different version LLVM with default in OS, not sure how about in github workflow, can try later. If all good, we can deprecate py37, WDYT?

2 options to upgrade ubuntu:

1. Install LLVM-12 in new ubuntu (not easy in OS, you may try with mac also)
2. Upgrade LLVM version in code (sounds better?) (but remember there's arguments change between LLVM 12 and 18)

For now, we can use ubuntu-20.04 or disable py37 to pass build right?

So you mean for ubuntu-22.04, LLVM-12 cannot be installed (I can't remember this, I thought any version of LLVM can be installed on any version of ubuntu...)? If that's the case, keeping ubuntu-20.04 (instead of using latest-ubuntu in the action workflow) might be the easiest solution.

[tancheng]

Hi @yyan7223, this PR may resolve some of the combinational loops issues. The combinational loops are identified by verilator itself via verilator CgraRTL__xxx__pickled.v --lint-only, e.g., Feedback to clock or circular logic: xxx.

Btw, @[email protected] (@[email protected]) and @yo96 would also help on this by some synthesis tool. If you cannot get report from OpenRoad, plz consider DC compiler.

[yuqisun]

@yuqisun python 3.7 doesn't exist on latest ubuntu (actions/runner-images#10636), seems like the new rule is starting being applied in our test workflow.
Do you think we need to keep ubuntu be 22.04, or deprecate python 3.7?

May keep ubuntu-20.04 (same as CGRA-Mapper) until get LLVM upgraded? Tried in OS, not easy to install different version LLVM with default in OS, not sure how about in github workflow, can try later. If all good, we can deprecate py37, WDYT?
2 options to upgrade ubuntu:

1. Install LLVM-12 in new ubuntu (not easy in OS, you may try with mac also)
2. Upgrade LLVM version in code (sounds better?) (but remember there's arguments change between LLVM 12 and 18)

For now, we can use ubuntu-20.04 or disable py37 to pass build right?

So you mean for ubuntu-22.04, LLVM-12 cannot be installed (I can't remember this, I thought any version of LLVM can be installed on any version of ubuntu...)? If that's the case, keeping ubuntu-20.04 (instead of using latest-ubuntu in the action workflow) might be the easiest solution.

Yes, think so, no luck when I try to install LLVM-12 in ubuntu-24.04 OS (default 18).

[tancheng]

@yuqisun python 3.7 doesn't exist on latest ubuntu (actions/runner-images#10636), seems like the new rule is starting being applied in our test workflow.
Do you think we need to keep ubuntu be 22.04, or deprecate python 3.7?

May keep ubuntu-20.04 (same as CGRA-Mapper) until get LLVM upgraded? Tried in OS, not easy to install different version LLVM with default in OS, not sure how about in github workflow, can try later. If all good, we can deprecate py37, WDYT?
2 options to upgrade ubuntu:

1. Install LLVM-12 in new ubuntu (not easy in OS, you may try with mac also)
2. Upgrade LLVM version in code (sounds better?) (but remember there's arguments change between LLVM 12 and 18)

For now, we can use ubuntu-20.04 or disable py37 to pass build right?

So you mean for ubuntu-22.04, LLVM-12 cannot be installed (I can't remember this, I thought any version of LLVM can be installed on any version of ubuntu...)? If that's the case, keeping ubuntu-20.04 (instead of using latest-ubuntu in the action workflow) might be the easiest solution.

Yes, think so, no luck when I try to install LLVM-12 in ubuntu-24.04 OS (default 18).

Thanks! No worry, I changed our default ubuntu version to 22.04 in the action workflow.

[yyan7223]

Hi cheng @tancheng

The attached file compiledc.log is the combinational loop report from DC compiler. Most of loops are on fu_crossbar and routing_crossbar. However, only 20 loops are reported, which is much less then 150 loops reported by Openroad.

By the way, I test verilator CgraRTL__xxx__pickled.v --lint-only on my side using docker image provided by Yuqi,
verilator automatically exits due to too many warning(s) and I cannot see any Feedback to clock or circular logic: xxx

So does DC compiler provide more details than Verilator on reporting combinational loops? If that can help us better resolve the combinational loops, I can further investigate why only 20 loops are reported in DC compiler.

regards,
yufei

[tancheng]

The attached file compiledc.log is the combinational loop report from DC compiler. Most of loops are on fu_crossbar and routing_crossbar. However, only 20 loops are reported, which is much less then 150 loops reported by Openroad.

Thanks Yufei! I saw all 20 loops are caused by SelRTL and I have updated/fixed in the latest commit, can you sync and try again? Hope other loops would be reported or the number of loops decreases.

By the way, I test verilator CgraRTL__xxx__pickled.v --lint-only on my side using docker image provided by Yuqi, verilator automatically exits due to too many warning(s) and I cannot see any Feedback to clock or circular logic: xxx

I have the exactly same msg as yours. And the Feedback to clock or circular logic: xxx appears when I scroll up my screen/terminal.

[yyan7223]

Cool, no loops reported by DC compiler now. @tancheng

[tancheng]

Cool, then let me merge this PR :-)