'primitive_type' attribute in verilog code is unsafe/bad style

[rdaly525]

The verilog logic relies on unsafe information like 'primitive_type' ('unary', 'binary', 'mux', 'other') that were originally just a way to efficiently generate verilog primitives. This should be changed to be calculated based off of the CoreIR Module's Type or changed to be an Enum.

[leonardt]

Can you explain more how you would do this given the type?

[rdaly525]

Something along these lines:

bool is_binary_type(Module* mod) {
  RecordType* rt = mod->getType();
  int num_bv_inputs = 0;
  int num_bv_outputs = 0;
  for (auto port : rt->getRecord()) {
     Type*  port_type = rt->select(port);
      if (port_type->isInput() && isBV(port_type)) {
         num_bv_inputs++;
      }
      //etc...
  }
  return num_bv_inputs ==2 && num_bv_outputs == 1;
}

Maybe I need to look at the code more, but if there are some 'standard' types needed for verilog inlining I would highly prefer dynamically determining them (more general) or specifying them using an Enum instead of a string.

[leonardt]

Makes sense

[leonardt]

I think we need to know a little more than just the number of inputs though, e.g. that the inputs are named a certain way (in0 and in1) since those symbols are used for doing the inline replacement of the connections.

[leonardt]

(But that can be encoded in the record type)