Skip to content

[ExportVerilog] Graph region op order creates redundant spills #8586

Open
@fabianschuiki

Description

@fabianschuiki

Consider the following two designs that only differ in their op order:

hw.module @FooA(in %in : i4, out out : i4) {
  %0 = comb.extract %in from 1 : (i4) -> i3
  %1 = comb.concat %2, %0 : i1, i3
  %2 = comb.extract %in from 0 : (i4) -> i1
  hw.output %1 : i4
}

hw.module @FooB(in %in : i4, out out : i4) {
  %0 = comb.extract %in from 1 : (i4) -> i3
  %1 = comb.extract %in from 0 : (i4) -> i1
  %2 = comb.concat %1, %0 : i1, i3
  hw.output %2 : i4
}

Running this through circt-opt --export-verilog produces the following:

module FooA(
  input  [3:0] in,
  output [3:0] out
);
  wire _in_0;
  assign _in_0 = in[0];
  assign out = {_in_0, in[3:1]};
endmodule

module FooB(
  input  [3:0] in,
  output [3:0] out
);
  assign out = {in[0], in[3:1]};
endmodule

I would expect both outputs to look like FooB, without a redundant spill when it isn't necessary. This probably has to do with PrepareForEmission or ExportVerilog going through the module's graph region in order and eagerly creating spills for use-before-def situations. It would be really nice if simple reorderings like this didn't affect emission quality, since the module is a graph region. Could we do a topological sort of the ops beforehand? Or have PrepareForEmission only insert spills for cycles or expressions that have multiple users?

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions