From 7d50a13f9f5a7266a8cd9ee536733677c861ac6d Mon Sep 17 00:00:00 2001
From: Denis Bernard <db047h@gmail.com>
Date: Sun, 24 May 2020 02:30:54 +0200
Subject: [PATCH] Decimal: forbid mantissae aliasing & improve Add/Sub
 performance

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.
---
 decimal.go | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/decimal.go b/decimal.go
index 63cc446..0327939 100644
--- a/decimal.go
+++ b/decimal.go
@@ -212,7 +212,7 @@ 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
@@ -220,13 +220,14 @@ func (z *Decimal) uadd(x, y *Decimal) {
 	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 {
@@ -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 {
@@ -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 {
@@ -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 {
@@ -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.