umulp is more delay intensive than regular umul

[proppy]

Describe the bug
umulp characterization show more delay than a * b for asap7.

Expected behavior

umulp should take less time that a*b

Screenshots
a * b yield the following delay preview:

umulp(a, b) yield the following delay preview:

[proppy]

asap7 8-bit umul:

xls/xls/estimators/delay_model/models/asap7.textproto

Lines 18732 to 18746 in 7e3d4e8

	data_points {
	operation {
	op: "kUMul"
	bit_count: 8
	operands {
	bit_count: 8
	}
	operands {
	bit_count: 8
	}
	specialization: OPERANDS_IDENTICAL
	}
	delay: 553
	delay_offset: 94
	}

asap7 8-bit umulp:

xls/xls/estimators/delay_model/models/asap7.textproto

Lines 19306 to 19320 in 7e3d4e8

	data_points {
	operation {
	op: "kUMulp"
	bit_count: 8
	operands {
	bit_count: 8
	}
	operands {
	bit_count: 8
	}
	specialization: OPERANDS_IDENTICAL
	}
	delay: 577
	delay_offset: 94
	}

[proppy]

Looks like this is the case too for sky130 too:

8-bit umul:

xls/xls/estimators/delay_model/models/sky130.textproto

Lines 18732 to 18746 in 7e3d4e8

	data_points {
	operation {
	op: "kUMul"
	bit_count: 8
	operands {
	bit_count: 8
	}
	operands {
	bit_count: 8
	}
	specialization: OPERANDS_IDENTICAL
	}
	delay: 1131
	delay_offset: 241
	}

8-bit umulp:

xls/xls/estimators/delay_model/models/sky130.textproto

Lines 19306 to 19320 in 7e3d4e8

	data_points {
	operation {
	op: "kUMulp"
	bit_count: 8
	operands {
	bit_count: 8
	}
	operands {
	bit_count: 8
	}
	specialization: OPERANDS_IDENTICAL
	}
	delay: 1181
	delay_offset: 241
	}

[proppy]

looks like this actually come from codegen: umulp is simply substracting a fix offset from the umul result:

  function automatic [15:0] umulp8b_8b_x_8b (input reg [7:0] lhs, input reg [7:0] rhs);
    reg [7:0] result;
    begin
      result = lhs * rhs;
      umulp8b_8b_x_8b = {8'h43, result - 8'h43};
    end
  endfunction

[proppy]

looks like this was introduced w/ af042f3#diff-a2fc2bd44970c5305b7655bcbce2d4e7f01015e489b78fe35bdc46dcfb3b00bd (and that before that one of the partial product was just 0).

[ericastor]

I think the change linked above was intended to be specific to simulation.

Possibly the actual issue is that XLS currently outputs its simulation-only stand-in for umulp when targeting a PDK that does not provide partial-multiplication operations.

[meheff]

Umulp enables splitting a multiply into a partial product (which produces two values) and the final accumulation. This can enable better scheduling because a pipeline register can appear in the middle of the multiply. However, this is only useful if you are using an IP libraries has a fast partial product and use the mulp_format flag in order to instantiate the IP block:

xls/docs_src/codegen_options.md

Line 274 in b959a4e

- `--smulp_format=...` and `--umulp_format=...` set the format strings for

Otherwise XLS just emits something like ((X * Y - C), C) for some constant C for the mulp which is not helpful. We probably want to emit an error if mulp is used without mulp_format being specified, assuming we don't do that already.