fix really old compiler bug (#799)

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.
+- 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.

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)
   )

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);

goalc/compiler/Util.cpp

@@ -303,4 +303,4 @@ void Compiler::compile_constant_product(const goos::Object& form,
     env->emit_ir<IR_LoadConstant64>(form, dest, stride);
     env->emit_ir<IR_IntegerMath>(form, IntegerMathKind::IMUL_32, dest, src);
   }
-}
+}

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<BlockEnv>(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<IR_RegSet>(form, ireg, reset_args_for_coloring.at(i));
     }
 

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;
   }
 

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<IR_RegSet>(form, ireg, reset_args_for_coloring.at(i));
   }
 

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;
   }

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
+        )

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) {