From ac3092093c6d259107d9e3cd7cff31f54569e6f0 Mon Sep 17 00:00:00 2001 From: water111 <48171810+water111@users.noreply.github.com> Date: Tue, 31 Aug 2021 22:12:30 -0400 Subject: [PATCH] fix really old compiler bug (#799) --- docs/markdown/progress-notes/changelog.md | 6 +++++- goal_src/engine/draw/process-drawable-h.gc | 2 +- goalc/compiler/IR.cpp | 2 ++ goalc/compiler/Util.cpp | 2 +- goalc/compiler/compilation/Function.cpp | 6 ++++-- goalc/compiler/compilation/Macro.cpp | 6 ++++++ goalc/compiler/compilation/Type.cpp | 5 ++++- goalc/emitter/IGen.h | 2 +- test/goalc/source_templates/with_game/tricky-floats.gc | 5 +++++ test/goalc/test_with_game.cpp | 10 ++++++++-- 10 files changed, 37 insertions(+), 9 deletions(-) create mode 100644 test/goalc/source_templates/with_game/tricky-floats.gc diff --git a/docs/markdown/progress-notes/changelog.md b/docs/markdown/progress-notes/changelog.md index 9cb746abee..0381b4e828 100644 --- a/docs/markdown/progress-notes/changelog.md +++ b/docs/markdown/progress-notes/changelog.md @@ -196,4 +196,8 @@ - Fixed a bug where nested rlet's didn't properly share register constraints, leading to inefficient register allocation, and some rare cases a regalloc constraint error - Lambdas may now be used in static pairs. - Dynamically constructed bitfields created with `(new 'static ...` may now set fields with `structure` type. -- Allocations on `'loading-level` are now permitted. \ No newline at end of file +- Allocations on `'loading-level` are now permitted. +- Converting a float larger than `INT32_MAX` now saturates to INT32_MAX, like on a real PS2. +- Treating a float as a 64-bit integer now sign extends, like on a real PS2 +- It is now an error to have two arguments with the same name. +- It is now a warning to redefine a constant. \ No newline at end of file diff --git a/goal_src/engine/draw/process-drawable-h.gc b/goal_src/engine/draw/process-drawable-h.gc index 7ca62835ff..ca4cbed664 100644 --- a/goal_src/engine/draw/process-drawable-h.gc +++ b/goal_src/engine/draw/process-drawable-h.gc @@ -42,7 +42,7 @@ ;; The following functions can be applied to a joint-control-channel to change the frame number. ;; They return the resulting frame number as well. -(defun num-func-none ((arg0 joint-control-channel) (arg2 float) (arg2 float)) +(defun num-func-none ((arg0 joint-control-channel) (arg1 float) (arg2 float)) "Don't change anything." (-> arg0 frame-num) ) diff --git a/goalc/compiler/IR.cpp b/goalc/compiler/IR.cpp index 1b1c509b69..f1bc790bc7 100644 --- a/goalc/compiler/IR.cpp +++ b/goalc/compiler/IR.cpp @@ -127,6 +127,8 @@ void regset_common(emitter::ObjectGenerator* gen, } else if (src_class == RegClass::FLOAT && dst_class == RegClass::GPR_64) { // xmm 1x -> gpr gen->add_instr(IGen::movd_gpr32_xmm32(dst_reg, src_reg), irec); + // don't forget to sign extend + gen->add_instr(IGen::movsx_r64_r32(dst_reg, dst_reg), irec); } else if (src_class == RegClass::GPR_64 && dst_class == RegClass::FLOAT) { // gpr -> xmm 1x gen->add_instr(IGen::movd_xmm32_gpr32(dst_reg, src_reg), irec); diff --git a/goalc/compiler/Util.cpp b/goalc/compiler/Util.cpp index 24e5b56180..5dbdcf3800 100644 --- a/goalc/compiler/Util.cpp +++ b/goalc/compiler/Util.cpp @@ -303,4 +303,4 @@ void Compiler::compile_constant_product(const goos::Object& form, env->emit_ir(form, dest, stride); env->emit_ir(form, IntegerMathKind::IMUL_32, dest, src); } -} \ No newline at end of file +} diff --git a/goalc/compiler/compilation/Function.cpp b/goalc/compiler/compilation/Function.cpp index 698695af0b..c5c7ac3fef 100644 --- a/goalc/compiler/compilation/Function.cpp +++ b/goalc/compiler/compilation/Function.cpp @@ -225,7 +225,6 @@ Val* Compiler::compile_lambda(const goos::Object& form, const goos::Object& rest place->func = new_func_env.get(); // nasty function block env setup - // TODO use calling convention auto return_reg = new_func_env->make_gpr(get_none()->type()); auto func_block_env = new_func_env->alloc_env(new_func_env.get(), "#f"); func_block_env->return_value = return_reg; @@ -236,7 +235,10 @@ Val* Compiler::compile_lambda(const goos::Object& form, const goos::Object& rest auto ireg = new_func_env->make_ireg( lambda.params.at(i).type, arg_regs.at(i).is_gpr() ? RegClass::GPR_64 : RegClass::INT_128); ireg->mark_as_settable(); - new_func_env->params[lambda.params.at(i).name] = ireg; + if (!new_func_env->params.insert({lambda.params.at(i).name, ireg}).second) { + throw_compiler_error(form, "lambda has multiple arguments named {}", + lambda.params.at(i).name); + } new_func_env->emit_ir(form, ireg, reset_args_for_coloring.at(i)); } diff --git a/goalc/compiler/compilation/Macro.cpp b/goalc/compiler/compilation/Macro.cpp index 579923e4de..f44332cd83 100644 --- a/goalc/compiler/compilation/Macro.cpp +++ b/goalc/compiler/compilation/Macro.cpp @@ -181,6 +181,12 @@ Val* Compiler::compile_define_constant(const goos::Object& form, "it is already the name of a symbol of type {}", sym->name, m_symbol_types.at(sym->name).print()); } + + auto existing = m_global_constants.find(sym); + if (existing != m_global_constants.end() && existing->second != value) { + print_compiler_warning("Constant {} has been redefined {} -> {}", sym->print(), + existing->second.print(), value.print()); + } m_global_constants[sym] = value; } diff --git a/goalc/compiler/compilation/Type.cpp b/goalc/compiler/compilation/Type.cpp index a0ef2229cc..3f3bc9edf0 100644 --- a/goalc/compiler/compilation/Type.cpp +++ b/goalc/compiler/compilation/Type.cpp @@ -522,7 +522,10 @@ Val* Compiler::compile_defmethod(const goos::Object& form, const goos::Object& _ auto ireg = new_func_env->make_ireg( lambda.params.at(i).type, arg_regs.at(i).is_gpr() ? RegClass::GPR_64 : RegClass::INT_128); ireg->mark_as_settable(); - new_func_env->params[lambda.params.at(i).name] = ireg; + if (!new_func_env->params.insert({lambda.params.at(i).name, ireg}).second) { + throw_compiler_error(form, "defmethod has multiple arguments named {}", + lambda.params.at(i).name); + } new_func_env->emit_ir(form, ireg, reset_args_for_coloring.at(i)); } diff --git a/goalc/emitter/IGen.h b/goalc/emitter/IGen.h index 595416d10f..60eec7dc44 100644 --- a/goalc/emitter/IGen.h +++ b/goalc/emitter/IGen.h @@ -2056,7 +2056,7 @@ class IGen { Instruction instr(0xf3); instr.set_op2(0x0f); instr.set_op3(0x2c); - instr.set_modrm_and_rex(dst.hw_id(), src.hw_id(), 3, true); + instr.set_modrm_and_rex(dst.hw_id(), src.hw_id(), 3, false); instr.swap_op0_rex(); return instr; } diff --git a/test/goalc/source_templates/with_game/tricky-floats.gc b/test/goalc/source_templates/with_game/tricky-floats.gc new file mode 100644 index 0000000000..9e8fe6005d --- /dev/null +++ b/test/goalc/source_templates/with_game/tricky-floats.gc @@ -0,0 +1,5 @@ +(format #t "#x~X ~f #x~X~%" + (the int 26843545000.0) ;; check that float -> int truncates (previously wrong) + (the float #x100000001) ;; check that int -> float truncates + (the-as int -1.0) ;; check that float reg -> int reg sign extends + ) \ No newline at end of file diff --git a/test/goalc/test_with_game.cpp b/test/goalc/test_with_game.cpp index 29d7aa4ea2..ae2d1b86fc 100644 --- a/test/goalc/test_with_game.cpp +++ b/test/goalc/test_with_game.cpp @@ -872,8 +872,14 @@ TEST_F(WithGameTests, GetEnumVals) { } TEST_F(WithGameTests, SetU64FromFloat) { - shared_compiler->runner.run_static_test(env, testCategory, "test-set-u64-from-float.gc", - {"-12.0000 #xc1400000 #xc1400000 #x0\n0\n"}); + shared_compiler->runner.run_static_test( + env, testCategory, "test-set-u64-from-float.gc", + {"-12.0000 #xffffffffc1400000 #xc1400000 #xffffffff\n0\n"}); +} + +TEST_F(WithGameTests, TrickyFloatBehavior) { + shared_compiler->runner.run_static_test(env, testCategory, "tricky-floats.gc", + {"#x80000000 1.0000 #xffffffffbf800000\n0\n"}); } TEST(TypeConsistency, TypeConsistency) {