Refactor some math functions into a shared file for rgbasm and rgblink

Fixes #769

Fixes #770

Makefile

@@ -72,7 +72,8 @@ rgbasm_obj := \
 	src/extern/getopt.o \
 	src/extern/utf8decoder.o \
 	src/hashmap.o \
-	src/linkdefs.o
+	src/linkdefs.o \
+	src/opmath.o
 
 src/asm/lexer.o src/asm/main.o: src/asm/parser.h
 
@@ -88,7 +89,8 @@ rgblink_obj := \
 	src/extern/err.o \
 	src/extern/getopt.o \
 	src/hashmap.o \
-	src/linkdefs.o
+	src/linkdefs.o \
+	src/opmath.o
 
 rgbfix_obj := \
 	src/fix/main.o \

include/opmath.h

@@ -0,0 +1,20 @@
+/*
+ * This file is part of RGBDS.
+ *
+ * Copyright (c) 1997-2021, RGBDS contributors.
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#ifndef RGBDS_OP_MATH_H
+#define RGBDS_OP_MATH_H
+
+#include <stdint.h>
+
+int32_t op_divide(int32_t dividend, int32_t divisor);
+int32_t op_modulo(int32_t dividend, int32_t divisor);
+int32_t op_exponent(int32_t base, uint32_t power);
+int32_t op_shift_left(int32_t value, int32_t amount);
+int32_t op_shift_right(int32_t value, int32_t amount);
+
+#endif /* RGBDS_OP_MATH_H */

src/CMakeLists.txt

@@ -60,6 +60,7 @@ set(rgbasm_src
     "extern/utf8decoder.c"
     "hashmap.c"
     "linkdefs.c"
+    "opmath.c"
     )
 
 set(rgbfix_src
@@ -83,6 +84,7 @@ set(rgblink_src
     "link/symbol.c"
     "hashmap.c"
     "linkdefs.c"
+    "opmath.c"
     )
 
 foreach(PROG "asm" "fix" "gfx" "link")

src/asm/rpn.c

@@ -25,6 +25,8 @@
 #include "asm/symbol.h"
 #include "asm/warning.h"
 
+#include "opmath.h"
+
 /* Makes an expression "not known", also setting its error message */
 #define makeUnknown(expr_, ...) do { \
 	struct Expression *_expr = expr_; \
@@ -244,77 +246,6 @@ void rpn_LOGNOT(struct Expression *expr, const struct Expression *src)
 	}
 }
 
-static int32_t shift(int32_t shiftee, int32_t amount)
-{
-	if (amount >= 0) {
-		// Left shift
-		if (amount >= 32) {
-			warning(WARNING_SHIFT_AMOUNT, "Shifting left by large amount %"
-				PRId32 "\n", amount);
-			return 0;
-
-		} else {
-			/*
-			 * Use unsigned to force a bitwise shift
-			 * Casting back is OK because the types implement two's
-			 * complement behavior
-			 */
-			return (uint32_t)shiftee << amount;
-		}
-	} else {
-		// Right shift
-		amount = -amount;
-		if (amount >= 32) {
-			warning(WARNING_SHIFT_AMOUNT,
-				"Shifting right by large amount %" PRId32 "\n", amount);
-			return shiftee < 0 ? -1 : 0;
-
-		} else if (shiftee >= 0) {
-			return shiftee >> amount;
-
-		} else {
-			/*
-			 * The C standard leaves shifting right negative values
-			 * undefined, so use a left shift manually sign-extended
-			 */
-			return (uint32_t)shiftee >> amount
-				| -(UINT32_C(1) << (32 - amount));
-		}
-	}
-}
-
-static int32_t divide(int32_t dividend, int32_t divisor)
-{
-	// Adjust division to floor toward negative infinity,
-	// not truncate toward zero
-	return dividend / divisor - ((dividend % divisor < 0) != (divisor < 0));
-}
-
-static int32_t modulo(int32_t dividend, int32_t divisor)
-{
-	int32_t remainder = dividend % divisor;
-
-	// Adjust modulo to have the sign of the divisor,
-	// not the sign of the dividend
-	return remainder + divisor * ((remainder < 0) != (divisor < 0));
-}
-
-static int32_t exponent(int32_t base, uint32_t power)
-{
-	int32_t result = 1;
-
-	for (;;) {
-		if (power % 2)
-			result *= base;
-		power /= 2;
-		if (!power)
-			break;
-		base *= base;
-	}
-
-	return result;
-}
-
 struct Symbol const *rpn_SymbolOf(struct Expression const *expr)
 {
 	if (!rpn_isSymbol(expr))
@@ -400,11 +331,17 @@ void rpn_BinaryOp(enum RPNCommand op, struct Expression *expr,
 					"Shifting left by negative amount %" PRId32 "\n",
 					src2->nVal);
 
-			expr->nVal = shift(src1->nVal, src2->nVal);
+			if (src2->nVal >= 32)
+				warning(WARNING_SHIFT_AMOUNT,
+					"Shifting left by large amount %" PRId32 "\n",
+					src2->nVal);
+
+			expr->nVal = op_shift_left(src1->nVal, src2->nVal);
 			break;
 		case RPN_SHR:
 			if (src1->nVal < 0)
-				warning(WARNING_SHIFT, "Shifting negative value %" PRId32 "\n",
+				warning(WARNING_SHIFT, "Shifting right negative value %"
+					PRId32 "\n",
 					src1->nVal);
 
 			if (src2->nVal < 0)
@@ -412,7 +349,12 @@ void rpn_BinaryOp(enum RPNCommand op, struct Expression *expr,
 					"Shifting right by negative amount %" PRId32 "\n",
 					src2->nVal);
 
-			expr->nVal = shift(src1->nVal, -src2->nVal);
+			if (src2->nVal >= 32)
+				warning(WARNING_SHIFT_AMOUNT,
+					"Shifting right by large amount %" PRId32 "\n",
+					src2->nVal);
+
+			expr->nVal = op_shift_right(src1->nVal, src2->nVal);
 			break;
 		case RPN_MUL:
 			expr->nVal = uleft * uright;
@@ -426,7 +368,7 @@ void rpn_BinaryOp(enum RPNCommand op, struct Expression *expr,
 					PRId32 "\n", INT32_MIN, INT32_MIN);
 				expr->nVal = INT32_MIN;
 			} else {
-				expr->nVal = divide(src1->nVal, src2->nVal);
+				expr->nVal = op_divide(src1->nVal, src2->nVal);
 			}
 			break;
 		case RPN_MOD:
@@ -436,7 +378,7 @@ void rpn_BinaryOp(enum RPNCommand op, struct Expression *expr,
 			if (src1->nVal == INT32_MIN && src2->nVal == -1)
 				expr->nVal = 0;
 			else
-				expr->nVal = modulo(src1->nVal, src2->nVal);
+				expr->nVal = op_modulo(src1->nVal, src2->nVal);
 			break;
 		case RPN_EXP:
 			if (src2->nVal < 0)
@@ -445,7 +387,7 @@ void rpn_BinaryOp(enum RPNCommand op, struct Expression *expr,
 			if (src1->nVal == INT32_MIN && src2->nVal == -1)
 				expr->nVal = 0;
 			else
-				expr->nVal = exponent(src1->nVal, src2->nVal);
+				expr->nVal = op_exponent(src1->nVal, src2->nVal);
 			break;
 
 		case RPN_UNSUB:

src/link/patch.c

@@ -18,82 +18,10 @@
 #include "link/symbol.h"
 
 #include "linkdefs.h"
+#include "opmath.h"
 
 #include "extern/err.h"
 
-static int32_t divide(int32_t dividend, int32_t divisor)
-{
-	// Adjust division to floor toward negative infinity,
-	// not truncate toward zero
-	return dividend / divisor - ((dividend % divisor < 0) != (divisor < 0));
-}
-
-static int32_t modulo(int32_t dividend, int32_t divisor)
-{
-	int32_t remainder = dividend % divisor;
-
-	// Adjust modulo to have the sign of the divisor,
-	// not the sign of the dividend
-	return remainder + divisor * ((remainder < 0) != (divisor < 0));
-}
-
-static int32_t exponent(int32_t base, uint32_t power)
-{
-	int32_t result = 1;
-
-	for (;;) {
-		if (power % 2)
-			result *= base;
-		power /= 2;
-		if (!power)
-			break;
-		base *= base;
-	}
-
-	return result;
-}
-
-static int32_t asl(int32_t value, int32_t shiftamt); // Forward decl for below
-static int32_t asr(int32_t value, int32_t shiftamt)
-{
-	uint32_t uvalue = value;
-
-	// Get the easy cases out of the way
-	if (shiftamt == 0)
-		return value;
-	if (value == 0 || shiftamt <= -32)
-		return 0;
-	if (shiftamt > 31)
-		return (value < 0) ? -1 : 0;
-	if (shiftamt < 0)
-		return asl(value, -shiftamt);
-	if (value > 0)
-		return uvalue >> shiftamt;
-
-	{
-		// Calculate an OR mask for sign extension
-		// 1->0x80000000, 2->0xC0000000, ..., 31->0xFFFFFFFE
-		uint32_t shiftamt_high_bits = -((uint32_t)1 << (32 - shiftamt));
-
-		return (uvalue >> shiftamt) | shiftamt_high_bits;
-	}
-}
-
-static int32_t asl(int32_t value, int32_t shiftamt)
-{
-	// Repeat the easy cases here to avoid INT_MIN funny business
-	if (shiftamt == 0)
-		return value;
-	if (value == 0 || shiftamt >= 32)
-		return 0;
-	if (shiftamt < -31)
-		return (value < 0) ? -1 : 0;
-	if (shiftamt < 0)
-		return asr(value, -shiftamt);
-
-	return (uint32_t)value << shiftamt;
-}
-
 /*
  * This is an "empty"-type stack. Apart from the actual values, we also remember
  * whether the value is a placeholder inserted for error recovery. This allows
@@ -251,7 +179,7 @@ static int32_t computeRPNExpr(struct Patch const *patch,
 				popRPN();
 				value = INT32_MAX;
 			} else {
-				value = divide(popRPN(), value);
+				value = op_divide(popRPN(), value);
 			}
 			break;
 		case RPN_MOD:
@@ -263,7 +191,7 @@ static int32_t computeRPNExpr(struct Patch const *patch,
 				popRPN();
 				value = 0;
 			} else {
-				value = modulo(popRPN(), value);
+				value = op_modulo(popRPN(), value);
 			}
 			break;
 		case RPN_UNSUB:
@@ -278,7 +206,7 @@ static int32_t computeRPNExpr(struct Patch const *patch,
 				popRPN();
 				value = 0;
 			} else {
-				value = exponent(popRPN(), value);
+				value = op_exponent(popRPN(), value);
 			}
 			break;
 
@@ -332,11 +260,11 @@ static int32_t computeRPNExpr(struct Patch const *patch,
 
 		case RPN_SHL:
 			value = popRPN();
-			value = asl(popRPN(), value);
+			value = op_shift_left(popRPN(), value);
 			break;
 		case RPN_SHR:
 			value = popRPN();
-			value = asr(popRPN(), value);
+			value = op_shift_right(popRPN(), value);
 			break;
 
 		case RPN_BANK_SYM:

src/opmath.c

@@ -0,0 +1,88 @@
+/*
+ * This file is part of RGBDS.
+ *
+ * Copyright (c) 1997-2021, RGBDS contributors.
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+/*
+ * Mathematical operators that don't reuse C's behavior
+ */
+
+#include <stdint.h>
+
+#include "opmath.h"
+
+int32_t op_divide(int32_t dividend, int32_t divisor)
+{
+	// Adjust division to floor toward negative infinity,
+	// not truncate toward zero
+	return dividend / divisor - ((dividend % divisor < 0) != (divisor < 0));
+}
+
+int32_t op_modulo(int32_t dividend, int32_t divisor)
+{
+	int32_t remainder = dividend % divisor;
+
+	// Adjust modulo to have the sign of the divisor,
+	// not the sign of the dividend
+	return remainder + divisor * ((remainder < 0) != (divisor < 0));
+}
+
+int32_t op_exponent(int32_t base, uint32_t power)
+{
+	int32_t result = 1;
+
+	for (;;) {
+		if (power % 2)
+			result *= base;
+		power /= 2;
+		if (!power)
+			break;
+		base *= base;
+	}
+
+	return result;
+}
+
+int32_t op_shift_left(int32_t value, int32_t amount)
+{
+	// Get the easy cases out of the way
+	if (amount == 0)
+		return value;
+	if (value == 0 || amount >= 32)
+		return 0;
+	if (amount < -31)
+		return (value < 0) ? -1 : 0;
+	if (amount < 0)
+		return op_shift_right(value, -amount);
+
+	// Use unsigned to force a bitwise shift
+	// Casting back is OK because the types implement two's complement behavior
+	return (uint32_t)value << amount;
+}
+
+int32_t op_shift_right(int32_t value, int32_t amount)
+{
+	// Repeat the easy cases here to avoid INT_MIN funny business
+	if (amount == 0)
+		return value;
+	if (value == 0 || amount <= -32)
+		return 0;
+	if (amount > 31)
+		return (value < 0) ? -1 : 0;
+	if (amount < 0)
+		return op_shift_left(value, -amount);
+
+	if (value > 0)
+		return (uint32_t)value >> amount;
+
+	// Calculate an OR mask for sign extension
+	// 1->0x80000000, 2->0xC0000000, ..., 31->0xFFFFFFFE
+	uint32_t amount_high_bits = -(UINT32_C(1) << (32 - amount));
+
+	// The C standard leaves shifting right negative values
+	// undefined, so use a left shift manually sign-extended
+	return ((uint32_t)value >> amount) | amount_high_bits;
+}

test/asm/shift.err

@@ -12,49 +12,41 @@ warning: shift.asm(16) -> shift.asm::test(6): [-Wshift-amount]
     Shifting left by large amount 32
 warning: shift.asm(17) -> shift.asm::test(3): [-Wshift-amount]
     Shifting left by negative amount -9001
-warning: shift.asm(17) -> shift.asm::test(3): [-Wshift-amount]
-    Shifting right by large amount 9001
 warning: shift.asm(17) -> shift.asm::test(6): [-Wshift-amount]
     Shifting left by negative amount -9001
-warning: shift.asm(17) -> shift.asm::test(6): [-Wshift-amount]
-    Shifting right by large amount 9001
 warning: shift.asm(19) -> shift.asm::test(3): [-Wshift]
-    Shifting negative value -1
+    Shifting right negative value -1
 warning: shift.asm(19) -> shift.asm::test(6): [-Wshift]
-    Shifting negative value -1
+    Shifting right negative value -1
 warning: shift.asm(20) -> shift.asm::test(3): [-Wshift]
-    Shifting negative value -1
+    Shifting right negative value -1
 warning: shift.asm(20) -> shift.asm::test(3): [-Wshift-amount]
     Shifting right by large amount 32
 warning: shift.asm(20) -> shift.asm::test(6): [-Wshift]
-    Shifting negative value -1
+    Shifting right negative value -1
 warning: shift.asm(20) -> shift.asm::test(6): [-Wshift-amount]
     Shifting right by large amount 32
 warning: shift.asm(21) -> shift.asm::test(3): [-Wshift]
-    Shifting negative value -1
+    Shifting right negative value -1
 warning: shift.asm(21) -> shift.asm::test(3): [-Wshift-amount]
     Shifting right by large amount 9001
 warning: shift.asm(21) -> shift.asm::test(6): [-Wshift]
-    Shifting negative value -1
+    Shifting right negative value -1
 warning: shift.asm(21) -> shift.asm::test(6): [-Wshift-amount]
     Shifting right by large amount 9001
 warning: shift.asm(22) -> shift.asm::test(3): [-Wshift]
-    Shifting negative value -4
+    Shifting right negative value -4
 warning: shift.asm(22) -> shift.asm::test(6): [-Wshift]
-    Shifting negative value -4
+    Shifting right negative value -4
 warning: shift.asm(23) -> shift.asm::test(3): [-Wshift]
-    Shifting negative value -4
+    Shifting right negative value -4
 warning: shift.asm(23) -> shift.asm::test(6): [-Wshift]
-    Shifting negative value -4
+    Shifting right negative value -4
 warning: shift.asm(24) -> shift.asm::test(3): [-Wshift]
-    Shifting negative value -1
+    Shifting right negative value -1
 warning: shift.asm(24) -> shift.asm::test(3): [-Wshift-amount]
     Shifting right by negative amount -9001
-warning: shift.asm(24) -> shift.asm::test(3): [-Wshift-amount]
-    Shifting left by large amount 9001
 warning: shift.asm(24) -> shift.asm::test(6): [-Wshift]
-    Shifting negative value -1
+    Shifting right negative value -1
 warning: shift.asm(24) -> shift.asm::test(6): [-Wshift-amount]
     Shifting right by negative amount -9001
-warning: shift.asm(24) -> shift.asm::test(6): [-Wshift-amount]
-    Shifting left by large amount 9001