handling structs thru conditionals should not introduce PPA penalties

[proppy]

Describe the bug

When handling structs/struct fields thru conditionals, we seems to pay an extra PPA penalty compared to testing the underlying flattened bits.

To Reproduce

with structs:

.x

import float32;

pub fn select_fp_struct(x: u32, y: u32, def: u32) -> u32 {
    let fx = float32::unflatten(x);
    let fy = float32::unflatten(y);
    let fdef = float32::unflatten(def);
    let res = if fx.bexp == u8:0 { fy } else { fdef };
    float32::flatten(res)
}

opt.ir

top fn select_fp_struct(x: bits[32], y: bits[32], def: bits[32]) -> bits[32] {
  fx_bexp: bits[8] = bit_slice(x, start=23, width=8)
  literal.0: bits[8] = literal(value=0)
  eq: bits[1] = eq(fx_bexp, literal.0)
  def_sign: bits[1]  = bit_slice(def, start=31, width=1)
  y_sign:   bits[1]  = bit_slice(y,   start=31, width=1)
  def_bexp: bits[8]  = bit_slice(def, start=23, width=8)
  y_bexp:   bits[8]  = bit_slice(y,   start=23, width=8)
  def_frac: bits[23] = bit_slice(def, start=0,  width=23)
  y_frac:   bits[23] = bit_slice(y,   start=0,  width=23)
  res_sign:     bits[1]  = sel(eq, cases=[def_sign, y_sign])
  res_bexp:     bits[8]  = sel(eq, cases=[def_bexp, y_bexp])
  res_fraction: bits[23] = sel(eq, cases=[def_frac, y_frac])
  ret concat: bits[32] = concat(res_sign, res_bexp, res_fraction)
}

.sv

module select_fp_struct(
  input wire [31:0] x,
  input wire [31:0] y,
  input wire [31:0] def,
  output wire [31:0] out
);
  wire res_sign;
  wire [7:0] res_bexp;
  wire [22:0] res_fraction;
  assign res_sign     = x[30:23] == 8'h00 ? y[31]    : def[31];
  assign res_bexp     = x[30:23] == 8'h00 ? y[30:23] : def[30:23];
  assign res_fraction = x[30:23] == 8'h00 ? y[22:0]  : def[22:0];
  assign out = {res_sign, res_bexp, res_fraction};
endmodule

with raw bits:

.x

import float32;

pub fn select_fp_bits(x: u32, y: u32, def: u32) -> u32 {
    let exp_x = x[23:31] as u8;
    if exp_x == u8:0 { y } else { def }
}

.opt.ir

top fn select_fp_bits(x: bits[32], y: bits[32], def: bits[32]) -> bits[32] {
  exp_x: bits[8] = bit_slice(x, start=23, width=8)
  literal.0: bits[8] = literal(value=0)
  eq: bits[1] = eq(exp_x, literal.0)
  ret sel: bits[32] = sel(eq, cases=[def, y])
}

.v

module select_fp_bits(
  input wire [31:0] x,
  input wire [31:0] y,
  input wire [31:0] def,
  output wire [31:0] out
);
  assign out = x[30:23] == 8'h00 ? y : def;
endmodule

Expected behavior

opt would lift:

concat(sel(bitslice, ...), sel(bitslice, ...), sel(bitslice, ...), ...);

into a top level sel:

sel(concat(bitslice, ...), concat(bitslice, ...), concat(bitslice, ...))

resulting into simpler optimized SystemVerilog output (and better PPA).

[ericastor]

Interesting! I'm also disappointed in the synthesis tools, if this makes a PPA difference... But this is still a thing XLS can improve.

[allight]

This seems like something we want to do really near the end of compilation I expect that we'd want to preserve the ability for intermediate optimizations to remove cases from the smaller concats since that seems likely to make a bigger difference then anything here.

[allight]

I also wonder if it might be worthwhile to create a pass which recognizes when we can shuffle bits around in order to create nodes which are identical or subsets of one another.