C++ Backend generates code with variables declared in wrong order

[zhemao]

Yunsup and I were doing some work on uncore and had some perfectly reasonable code work fine in vsim but cause a C++ compile error in the emulator.

/scratch/zhemao/rocket-chip/emulator/generated-src/Top.DefaultCPPConfig-1.cpp:1587:22: error: ‘T718’ was not declared in this scope
   { T717 = TERNARY_1(T718, T716, T714);}

Since the temp variables are declared and assigned in order, it's obvious that the line of generated code above is incorrect. T718 is declared after T717 is assigned.

Steps to reproduce: Check out the latest rocket-chip master and switch to the "tl-fixes" branch on uncore. Try to compile the emulator and you'll see a whole host of similar errors.

[ucbjrl]

Verified.

[zhemao]

It turns out that this issue was due to a combinational loop in the RTL. But this combinational loop was not detected by Rocket. So now the question is, why was the combinational loop not detected by the Chisel compiler?

[ccelio]

Can you break it down into a smaller scenario? Was it through the I/O bundle? Sounds like a very important bug!

[zhemao]

Basically, if you take two different arbiters, and make the valid signal of each one dependent on the ready signal of the other, you will get a combinational loop, since internally, the arbiter ready signals are dependent on the valid signals.

[donggyukim]

Unfortunately, the pass to find a comb loop was broken. I'm fixing it.