Potentially incorrect execution with negative zeros

[mysoreanoop]

Regarding: https://github.com/ucb-bar/berkeley-hardfloat/blob/9deaf1d49f487347be371641ba6ea637b669ad21/src/main/scala/MulAddRecFN.scala#L285

Problem: In cases where FMA unit is used for pure multiplication (by basically having C=+0 in A*B+C of FMA), sometimes, the sign of the output (against intention) depends on the sign of C (it shouldn't because the intention is to only do multiplication, and not FMA).

Proposed solution: Have a wire input into MulAddRecFN to indicate if an operation is pure multiplication, in which case, the line linked above needs to be changed to propagate only sign of (A*B), and not consider the influence of sign of C.

If you think this is a decent fix, I can do a PR, otherwise, if you expect this condition needs to be handled externally (by a pipeline), then, I can put in a comment or update the doc somewhere.

[nibrunieAtSi5]

I think this is a well known issue when using a FMA to execute FMUL only and is generally handled by an external wrapper. (Some people do not need / want to handle FMUL in an FMA and so the extra input should be optional and it makes more sense to have a proper FMA with the extra complexity and let people handle FMUL during the integration).

[mysoreanoop]

Thanks for the reply, that makes sense. I'll close the issue.

[nibrunieAtSi5]

You may want to wait for others to chip in (I don't pretend to have the definitive answer on your relevant question).

[aamartin0000]

In my wrapper code that steers/modifies the source operands, the C operand can be a signed zero, the sign bit modified as needed. Determining the use of negative zero is not much logic, and has minimal impact to the timing.

An extra input, or any modification of mulAddRecFN, for this case is not needed.

[mysoreanoop]

I see. So, you determine C based on the sign of A * B (and the rounding mode) that you recompute in your wrapper? We ended up going with the HardFloat modification -- do you see any obvious issue with it?

[nibrunieAtSi5]

I think the sign of C=0 only depends on the rounding mode as -0is the identity for addition for anything but rounding down (rounding towards minus infinity) and +0 is the identity of the addition when rounding down.

[mysoreanoop]

If AB compute to 0 or -0, and that is all the operation is, then sign of C depends on sign of AB and the rounding mode -- right?

[nibrunieAtSi5]

I don't think so,

• if A.B=+0
  • if rounding mode is down, then C=+0 and A.B + C = +0 + +0 = +0 (Ok)
  • if rounding mode is not down, then C=-0 and A.B + C = +0 + -0 = +0 (Ok, as +0 + -0 = +0 when not rounding down)
• if A.B=-0
  • if rounding mode is down, then C=+0 and A.B + C = -0 + +0 = -0 (Ok, , as -0 + +0 = -0 in rounding down)
  • if rounding mode is not down, then C=-0 and A.B + C = -0 + -0 = -0 (Ok, as -0 + -0 = -0 when not rounding down)

so C does not depend on sign of A.B just on rounding mode.

[aamartin0000]

IMHO, this modification should be local to your implementation, and not released to the main code branch. My wrapper around mulAddRecFN involves steering/modifying the operands for fadd/fsub, NaN boxing, and different sized operands (e.g. using a single instance for dp and sp). No modifications to mulAddRecFN, other than to add pipeline flops.

Having said that, my starting point was the Verilog code, not the scala code. So even if you change the Scala, I won't need to make any local changes.

[mysoreanoop]

@nibrunieAtSi5 Ah, yes, that makes sense, thank you for the explanation (typo in the 4th case for posterity). @aamartin0000 I agree, that makes sense, thanks.

[nibrunieAtSi5]

(Thx, posterity is important)

[aamartin0000]

In my wrapper, the code is essentially sZero = (rs1 != rs2) ? neg_zero : pos_zero

and then this is chosen for "C" if it's fmul. Although doing a full comparison is somewhat expensive, its latency is buried by "fNToRecFN" source operand recoding being done in parallel. Note that the equation doesn't include the rounding mode.