Pass to remove combinational groups

[rachitnigam]

Slicing this pass out of #589 because this should be standalone.

The newly added remove-comb-groups pass transforms all groups with a done
condition in the form of group[done] = 1'd1 to groups that take exactly one
cycle. In the future, this pass will do the same thing to groups defined using
the the custom comb group syntax (#588).

The works as follows:

1. Walk over the control AST and look for all instances of comb groups used
   using the with syntax of if and while.
2. When such an instance is found, remember the port being read.
3. Rewrite the group to save the value of all such ports into registers.
4. Rewrite uses of ports in the control program with that of the corresponding
   registers.

This PR also makes it a hard error to use a combinational group within normal
control operartors such as seq and par. Combinational groups may only be
used as part of the with syntax.

[rachitnigam]

Runs into the bug with #520.

[rachitnigam]

@cgyurgyik the TCAM test seems to fail on this PR. Can I get some help debugging what is going/what is expected?

[rachitnigam]

Changes from #624 to the tcam implementation doesn't seem to fix the problem.

[rachitnigam]

Aha! Turned out to be a dump problem---infer-static-timing was running before remove-comb-groups which changes the "static" annotations on groups but cannot transitively propagate that to other inferred latencies. This does create an annoying restriction: a component with a top-level "static" annotation cannot have combinational groups. I don't have any good solutions to this problem.

[rachitnigam]

I think this opens up a deeper question about passes that change the timing behavior of components. In essence, by using both a combinational group, you're telling the compiler to automatically schedule it while by giving it a "static" annotation, you're telling it that the component has a specific timing behavior. These two things are in conflict.

Similar problems will occur with level-2 primitives like pipelines which imply register insertion and might have different timing behavior.

[sampsyo]

Got it. Is it a correct summary to say that:

• The timing information was inferred assuming that combinational groups take zero cycles.
• Then, remove-comb-groups can, at least sometimes, make these groups into sequential groups that use >=1 cycle.
• Now the inferred "static" annotations are wrong.

Surely there is something deeper to be done here, but it seems like a reasonable low-hanging-fruit solution would be:

• Make it a policy that combinational groups take an unknown amount of time.
• Therefore, infer-static-timing must not succeed when it analyzes code that uses combinational groups. (That is, it must not infer an annotation.)

Then the practical thing to do, as you implied, is to make infer-static-timing come later in the pipeline than remove-comb-groups so it can "succeed" as often as possible. (But even if it runs earlier in the pipeline for some reason, at least it's not wrong.)

[sampsyo]

All looks good! Wahoo!!

[EclecticGriffin]

LGTM!