From ac2cefdd8765e3daafea8eba405fc50e63626aaa Mon Sep 17 00:00:00 2001 From: Rangi Date: Sun, 28 Feb 2021 16:50:47 -0500 Subject: [PATCH] Refactor some math functions into a shared file for rgbasm and rgblink Fixes #769 Fixes #770 --- Makefile | 6 ++- include/opmath.h | 20 ++++++++++ src/CMakeLists.txt | 2 + src/asm/rpn.c | 96 +++++++++------------------------------------- src/link/patch.c | 84 +++------------------------------------- src/opmath.c | 88 ++++++++++++++++++++++++++++++++++++++++++ test/asm/shift.err | 32 ++++++---------- 7 files changed, 151 insertions(+), 177 deletions(-) create mode 100644 include/opmath.h create mode 100644 src/opmath.c diff --git a/Makefile b/Makefile index b6929666..7e4f82c6 100644 --- a/Makefile +++ b/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 \ diff --git a/include/opmath.h b/include/opmath.h new file mode 100644 index 00000000..0c83b83c --- /dev/null +++ b/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 + +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 */ diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 88c6e867..b72eb890 100644 --- a/src/CMakeLists.txt +++ b/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") diff --git a/src/asm/rpn.c b/src/asm/rpn.c index d02f7449..3abd7ad9 100644 --- a/src/asm/rpn.c +++ b/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: diff --git a/src/link/patch.c b/src/link/patch.c index c7fe78df..707a5114 100644 --- a/src/link/patch.c +++ b/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: diff --git a/src/opmath.c b/src/opmath.c new file mode 100644 index 00000000..d1f6e335 --- /dev/null +++ b/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 + +#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; +} diff --git a/test/asm/shift.err b/test/asm/shift.err index ba1c7543..9bb4109d 100644 --- a/test/asm/shift.err +++ b/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