Semantics of Component Inlining/ Possible Bug in Component Inlining

[calebmkim]

Example

The following program runs differently when we inline vs. don't inline (it's kinda complicated sorry, and here are the full files a.futil.txt, a.json.txt):

// component that adds in and in2 and stores the result in out_val 
// constant_on is always 1 
component adder(in: 32, in2: 32) -> (out_val: 32, constant_on: 1) {
  cells {
    a_reg = std_reg(32);
  }
  wires {
    // adds in and in2, stores result in a_reg
    group add {...}
    out_val = a_reg.out;
    constant_on = 1'd1;
  }
  control {
    add; 
  }
}

component main() -> () {
  cells {
    a = adder();
    main_reg = std_reg(32);
    @external m = std_mem_d1(32,1,1);
  }
  wires {
     // in 2 cycles, write main_reg.out into memory m 
     static<2> group write_mem {
      m.write_data = main_reg.out;
      m.write_en = 1'd1; 
      m.addr0 = 1'd0;
    }
  }
  control {
    seq {
      invoke a(in = 32'd2, in2 = 32'd4)(out_val = main_reg.in, constant_on = main_reg.write_en);
      write_mem;
    }
  }
}

With inlining:
fud e --to dat --through icarus-verilog -s verilog.data a.json a.futil --from calyx -s calyx.flags "-x inline:always"
m ends up with value 6

Without inlining
fud e --to dat --through icarus-verilog -s verilog.data a.json a.futil --from calyx
m comes out with value 0.

Explanation

After some investigating, the problem it seems is that in invoke a(...)(...); the connections inside the (...) should only be active when the invoke is active (I believe). However, the way inlining currently works is that it adds a wire a_in, a_in2 etc. for each port in a and a continuous assignment that assigns to a_in as a continuous assignment in the main component. The problem is, that we don't want a_in to be assigned continuously in the main component. We only want it assigned when the invoke is happening.

One bright side I discovered this when I was testing inlining heuristics and I was running into a limitation in inlining: namely, we cannot invoke instances when the bindings are different (e.g., if we invoke invoke a(...)(...); in main twice, but the bindings inside (...) are different, then inlining doesn't work). The reason for this, I believe, is because the continuous assignments to a_in, a_in2, etc. can't conflict, but if there are different bindings for different invokes, then they would. So if we solve this bug, then we would also probably solve this limitation of inlining as well.

Conclusion

It almost seems like when we inline invoke a(...)(...), it should be replaced by:
par { //a's control flow; // the assignments inside (...)(...) }
Also, lmk if this doesn't make sense and I can try to clarify. Or perhaps a synchronous discussion would be helpful.

[rachitnigam]

Interesting! I haven't read the example carefully yet but one passing thought about the new proposed control program: I think it somewhat makes sense but the big question is how do you build a group to enable the assignments in (...)(...)? The par control program does not guarantee that groups will be active at the same time so not sure we'll get the semantics we want.

On the limitation with different bindings: Yup, this is a known limitation that this is because we don't have a clear way to inline different assignments to the control program of the invoked component when it is used with different bindings. Two possible approaches are:

1. Defining a more general with operator (which will make analysis of active connections more complicated): The `with` operator #934
2. Defining different versions of the control program of the invoked program for each binding

The discussion in the inlining PR and the original issue might shed more light on the challenges: #829

[rachitnigam]

I would also suggest running the design with Verilator to see if it still produces the wrong answer

[calebmkim]

Thanks for the response. Running the design with Verilator produces the same inconsistency in results.

I think it somewhat makes sense but the big question is how do you build a group to enable the assignments in (...)(...)? The par control program does not guarantee that groups will be active at the same time so not sure we'll get the semantics we want.

Yeah, I'm not sure how we would make assignments active for the exact same time as the component's control program. Of course, if the control program is static, then it would be quite easy: we could just define a static group with a latency equal to that of the control program that contains the assignments we want. But that's not a solution in general.

For dynamic control, I can't think of a good solution right now.
Just so we have it in writing, the case I'm having trouble handling is the following:
What if we have a

comp (...)(out:32) {
   wires {
        // continuous assignment 
        out = reg.out 
   }
} 

If, in main, we have

seq {
    invoke comp(...)(out = reg1.in)
    invoke comp(...)(out = reg2.in)
} 

How do we make sure that comp.out = reg1.in is active for the first inlined invoke, but comp.out = reg2.in for the second invoke?

[rachitnigam]

@calebmkim we should probably address this as a part of #1813?

[calebmkim]

Sounds good-- I thin this shouldn't be too difficult if we are end up implementing a general with operator.