Skip to content

Commit

Permalink
Decimal: forbid mantissae aliasing & improve Add/Sub performance
Browse files Browse the repository at this point in the history
We need to distinguish mantissa aliasing, the test performed by the
alias() function, from "same" mantissa, the test performed by the same()
function. Mantissa aliasing is not to be confused with Decimal argument
aliasing either. Decimal aliasing happens when calling for example
z.Add(z, foo): within the (z *Decimal).Add(x, y *Decimal) function,
argument z will alias x.

In uadd/usub, there was a check

    al := alias(z.mant, x.mant) || alias(z.mant, y.mant)

that suggested that the mantissa of some Decimal could "alias" the
mantissa of another. i.e. that the following could happen: z.mant =
x.mant[n:] (where n > 0). This is not desirable: some dec functions
handle aliasing gracefuly but under some unchecked or undocumented
conditions. For example (dec).shl is written to support z.shl(z, foo)
but not z.shl(z[n:], foo).

The first part of this change is to forbid mantissa aliasing (but not
"same" mantissa, i.e. Decimal aliasing).

The second part is in uadd/usub: the alias() calls are replaced by
same() calls, and instead of making a temp copy if either same(z.mant,
x.mant) or same(z.mant, y.mant) is true, the copy is mode if only the
relevant condition is true. With mantissa aliasing forbidden, there is
no need to check the arguments of the z.shl() calls.
  • Loading branch information
db47h committed May 24, 2020
1 parent ceef8eb commit 7d50a13
Showing 1 changed file with 25 additions and 12 deletions.
37 changes: 25 additions & 12 deletions decimal.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,21 +212,22 @@ func (z *Decimal) uadd(x, y *Decimal) {
// http://www.vinc17.net/research/papers/rnc6.pdf

if debugDecimal {
validateBinaryOperands(x, y)
validateTernaryOperands(z, x, y)
}

// compute exponents ex, ey for mantissa with decimal point
// on the right (mantissa.0) - use int64 to avoid overflow
ex := int64(x.exp) - int64(len(x.mant))*_DW
ey := int64(y.exp) - int64(len(y.mant))*_DW

al := alias(z.mant, x.mant) || alias(z.mant, y.mant)

// TODO(gri) having a combined add-and-shift primitive
// could make this code significantly faster
// TODO(db47h) having a combined add-and-shift primitive
// could make this code significantly faster
// but this needs a version of shl that starts
// from the least significant word and forbids
// in-place shifts.
switch {
case ex < ey:
if al {
if same(z.mant, x.mant) {
t := dec(nil).shl(y.mant, uint(ey-ex))
z.mant = z.mant.add(x.mant, t)
} else {
Expand All @@ -237,7 +238,7 @@ func (z *Decimal) uadd(x, y *Decimal) {
// ex == ey, no shift needed
z.mant = z.mant.add(x.mant, y.mant)
case ex > ey:
if al {
if same(z.mant, y.mant) {
t := dec(nil).shl(x.mant, uint(ex-ey))
z.mant = z.mant.add(t, y.mant)
} else {
Expand All @@ -261,17 +262,15 @@ func (z *Decimal) usub(x, y *Decimal) {
// by special-casing, and the code will diverge.

if debugDecimal {
validateBinaryOperands(x, y)
validateTernaryOperands(z, x, y)
}

ex := int64(x.exp) - int64(len(x.mant))*_DW
ey := int64(y.exp) - int64(len(y.mant))*_DW

al := alias(z.mant, x.mant) || alias(z.mant, y.mant)

switch {
case ex < ey:
if al {
if same(z.mant, x.mant) {
t := dec(nil).shl(y.mant, uint(ey-ex))
z.mant = t.sub(x.mant, t)
} else {
Expand All @@ -282,7 +281,7 @@ func (z *Decimal) usub(x, y *Decimal) {
// ex == ey, no shift needed
z.mant = z.mant.sub(x.mant, y.mant)
case ex > ey:
if al {
if same(z.mant, y.mant) {
t := dec(nil).shl(x.mant, uint(ex-ey))
z.mant = t.sub(t, y.mant)
} else {
Expand Down Expand Up @@ -1459,6 +1458,20 @@ func validateBinaryOperands(x, y *Decimal) {
if len(y.mant) == 0 {
panic("empty mantissa for y")
}
// disallow aliasing of mantissae. i.e. prevent things like z.mant = x.mant[m:n]
if alias(x.mant, y.mant) && !same(x.mant, y.mant) {
panic("x.mant is an alias for y.mant")
}
}

func validateTernaryOperands(z, x, y *Decimal) {
validateBinaryOperands(x, y)
if alias(z.mant, y.mant) && !same(z.mant, y.mant) {
panic("z.mant is an alias for y.mant")
}
if alias(z.mant, x.mant) && !same(z.mant, x.mant) {
panic("z.mant is an alias for x.mant")
}
}

// round rounds z according to z.mode to z.prec digits and sets z.acc accordingly.
Expand Down

0 comments on commit 7d50a13

Please sign in to comment.