calyxir / calyx

Intermediate Language (IL) for Hardware Accelerator Generators
https://calyxir.org
MIT License
450 stars 44 forks source link

`@data` carried over to external memories #2019

Closed calebmkim closed 3 weeks ago

calebmkim commented 3 weeks ago

@andrewb1999 does this solve #2012? Lmk if it doesn't.

Here's why I think it wasn't working before:

The Problem

The problem was that, in order to have "default-to-zero behavior" we needed the cell and the port to be marked as@data. However, when we externalize it, we change the parent cell.

component main(@go go: 1, @clk clk: 1, @reset reset: 1) -> (@done done: 1) {
  cells {
    // out.write_data's parent is out, which is marked as @data
    @external @data out = seq_mem_d1(32, 1, 1);
    ...
} 
// out_write_data's parent is now main which is not marked as @data 
component main(...@data out_write_data: 32....) { ... }

The Solution

I made a pretty hacky solution: when we externalize a cell, we only add the @data attribute when the cell is @data as well. This way, when we're in the Verilog backend, if we detect the parent is this_component, then we don't have to check that it's parent is a @data cell-- we already checked that during the externalize pass. Also, we need to mark ports that we externalized with an @externalize attribute-- it's only these @externalize ports that we can "skip" the step of checking the parent.

andrewb1999 commented 3 weeks ago

Looks like that fixes it! Thanks for doing this so quickly.

calebmkim commented 3 weeks ago

Sounds good: I will merge rn and then address any comments retroactively.

rachitnigam commented 3 weeks ago

@calebmkim this is a bit too hacky for my taste. Attributes like @externalized create implicit dependencies between passes in a way that's hard to understand. Is there a different solution that could work? Can we discuss the exact problem in more detail using some examples in an issue?

If not, we should make @externalized an internal attribute at the very least.

calebmkim commented 3 weeks ago

I'm realizing another possible way of "solving" this is: if @data is an output port in the signature of ThisComponent, then we know this is the result of an the externalize pass, right? In every other case, @data goes on the input port of the signature. If this is the case, then I think the attribute I added (@externalized) is unnecessary-- we can just check whether it is an input vs. output port in the signature.

(Edit: I don't even think we have to check whether it is an input port or output port-- the mere fact that we are writing into a ThisComponent.port means that it is an output port on the signature-- meaning that if it has the @data attribute, then it must have come from the externalize pass).

(Edit 2: I expanded on the exact problem here if anyone is interested in reading up on it: https://github.com/calyxir/calyx/issues/2012#issuecomment-2075721509)

Doest this make sense?

rachitnigam commented 3 weeks ago

Why cant a user generate a data annotation on an output port of a component?

calebmkim commented 3 weeks ago

Sorry, I may have been wrong about that. But the distinction I am trying to draw is that there are 2 possible meanings of the @data annotation.

  1. For "external consumption". This is the case for most primitives, but could also be applied to Calyx components. For example, we define an adder that has two @data ports.
    comb primitive std_fp_add<"share"=1>[
    WIDTH, INT_WIDTH, FRAC_WIDTH
    ](@data left: WIDTH, @data right: WIDTH) ->(out: WIDTH); 

Then, we instantiate the adder.

add = std_fp_add(32, 16, 16). 

Now, any assignments to add.left get treated as @data ports (with "default to x" semantics), provided add is an @data cell.

  1. For "internal consumption". This can only apply to Calyx components (this is the case that occurs when we externalize memories).
    For example, we define a Calyx component.
    component main() ->(...@data out_write_data: 32....) { 
    // any assignment to out_write_data gets "default to x" semantics
    }

    Then, we get "default to x" semantics when assigning to the port itself inside of the component, not an instantiation of main. In this case, we don't need the parent (in this case, main to be marked as @data.)

Note that in (1), the port must appear on the input port of the cell's signature and it has the extra requirement that the parent must be an @data cell. It seems that in (2), the port must appear on the output port of the cell's signature and does not require that the parent be a @data cell.

Does that make sense?

rachitnigam commented 3 weeks ago

This can only apply to Calyx components (this is the case that occurs when we externalize memories)

I guess this is the part I don't understand. I think you can write @data on output ports on primitives as well. Maybe you're pointing out that we haven't really defined what that means?

I think I see the problem: we need to know that an output port is marked @data so that within the component (where we generate assignments for it), we can generate an 'x value.

I think maybe the solution is updating the semantics of @data to talk about output ports and say how it affects the generation of default assignments of Calyx component output ports. Then, the guarantee is that if the user generates a @data output port, within the component, we will generate 'x assignments.

calebmkim commented 3 weeks ago

I guess this is the part I don't understand. I think you can write @data on output ports on primitives as well. Maybe you're pointing out that we haven't really defined what that means?

Yeah, that's what I mean: you can technically write @data wherever you want (I guess you could even put it on groups/control if you wanted) but I don't think it has any real meaning when you write @data on an output port of a primitive.

I think I see the problem: we need to know that an output port is marked @data so that within the component (where we generate assignments for it), we can generate an 'x value.

Yep, that's my understanding of the problem! I think we are on the same page now.

I think maybe the solution is updating the semantics of @data to talk about output ports and say how it affects the generation of default assignments of Calyx component output ports. Then, the guarantee is that if the user generates a @data output port, within the component, we will generate 'x assignments.

Yeah, this sounds good-- should be easy. I'll do that.