diff --git a/doc/changelog.md b/doc/changelog.md index d88678c245..814c211a1a 100644 --- a/doc/changelog.md +++ b/doc/changelog.md @@ -103,4 +103,7 @@ - Improved back up and restore of xmm registers - Fixed an off-by-one in move eliminator (previous version was correct, but did not generate as good code). Complicated functions are 2 to 10% smaller. - Improved getting a stack address. -- Improved getting the value of `#f`, `#t`, and `()`. \ No newline at end of file +- Improved getting the value of `#f`, `#t`, and `()`. +- Accessing a constant field of an array now constant propagates the memory offset like field access and avoids a runtime multiply. +- Fixed a bug where loading or storing a `vf` register from a memory location + constant offset would cause the compiler to throw an error. +- Accessing array elements uses more efficient indexing for power-of-two element sizes. \ No newline at end of file diff --git a/goalc/compiler/Compiler.h b/goalc/compiler/Compiler.h index 9cb6e28a98..9493367d32 100644 --- a/goalc/compiler/Compiler.h +++ b/goalc/compiler/Compiler.h @@ -237,6 +237,7 @@ class Compiler { StaticStructure* structure, int offset, Env* env); + void compile_constant_product(RegVal* dest, RegVal* src, int stride, Env* env); template void throw_compiler_error(const goos::Object& code, const std::string& str, Args&&... args) { diff --git a/goalc/compiler/Util.cpp b/goalc/compiler/Util.cpp index f396b060b4..5c4ab22813 100644 --- a/goalc/compiler/Util.cpp +++ b/goalc/compiler/Util.cpp @@ -2,6 +2,9 @@ #include "goalc/compiler/IR.h" #include "common/goos/ParseHelpers.h" +/*! + * Parse arguments into a goos::Arguments format. + */ goos::Arguments Compiler::get_va(const goos::Object& form, const goos::Object& rest) { goos::Arguments args; @@ -12,6 +15,10 @@ goos::Arguments Compiler::get_va(const goos::Object& form, const goos::Object& r return args; } +/*! + * Check arguments in a goos::Arguments format (named and unnamed) and throw a compiler error if it + * fails. + */ void Compiler::va_check( const goos::Object& form, const goos::Arguments& args, @@ -24,6 +31,10 @@ void Compiler::va_check( } } +/*! + * Iterate through elements of a goos list and apply the given function. Throw compiler error if the + * list is invalid. + */ void Compiler::for_each_in_list(const goos::Object& list, const std::function& f) { const goos::Object* iter = &list; @@ -38,14 +49,24 @@ void Compiler::for_each_in_list(const goos::Object& list, } } +/*! + * Convert a goos::Object that's a string to a std::string. Must be a string. + */ std::string Compiler::as_string(const goos::Object& o) { return o.as_string()->data; } +/*! + * Convert a goos::Object that's a symbol to a std::string. Must be a string. + */ std::string Compiler::symbol_string(const goos::Object& o) { return o.as_symbol()->name; } +/*! + * Convert a single quoted symbol into a std::string. Like 'hi -> "hi". Error if not a quoted + * symbol. + */ std::string Compiler::quoted_sym_as_string(const goos::Object& o) { auto args = get_va(o, o); va_check(o, args, {{goos::ObjectType::SYMBOL}, {goos::ObjectType::SYMBOL}}, {}); @@ -55,6 +76,9 @@ std::string Compiler::quoted_sym_as_string(const goos::Object& o) { return symbol_string(args.unnamed.at(1)); } +/*! + * Get a thing that's quoted. Error if the thing isn't quoted. + */ goos::Object Compiler::unquote(const goos::Object& o) { auto args = get_va(o, o); va_check(o, args, {{goos::ObjectType::SYMBOL}, {}}, {}); @@ -64,6 +88,9 @@ goos::Object Compiler::unquote(const goos::Object& o) { return args.unnamed.at(1); } +/*! + * Determine if o is a quoted symbol like 'test. + */ bool Compiler::is_quoted_sym(const goos::Object& o) { if (o.is_pair()) { auto car = pair_car(o); @@ -255,4 +282,27 @@ std::vector Compiler::get_list_as_vector(const goos::Object& o, return result; } } +} + +void Compiler::compile_constant_product(RegVal* dest, RegVal* src, int stride, Env* env) { + // todo - support imul with an imm. + assert(stride); + + bool is_power_of_two = (stride & (stride - 1)) == 0; + if (stride == 1) { + env->emit_ir(dest, src); + } else if (is_power_of_two) { + for (int i = 0; i < 16; i++) { + if (stride == (1 << i)) { + env->emit_ir(dest, src); + env->emit_ir(IntegerMathKind::SHL_64, dest, i); + return; + } + } + assert(false); + } else { + // get the multiplier + env->emit_ir(dest, stride); + env->emit_ir(IntegerMathKind::IMUL_32, dest, src); + } } \ No newline at end of file diff --git a/goalc/compiler/Val.cpp b/goalc/compiler/Val.cpp index ccbc722b8b..4175b3a52c 100644 --- a/goalc/compiler/Val.cpp +++ b/goalc/compiler/Val.cpp @@ -109,13 +109,33 @@ RegVal* FloatConstantVal::to_reg(Env* fe) { return re; } +namespace { +/*! + * Constant propagate nested MemoryOffsetConstantVal's to get a single base + offset. + */ +Val* get_constant_offset_and_base(MemoryOffsetConstantVal* in, int64_t* offset_out) { + Val* next_base = in->base; + s64 total_offset = in->offset; + while (dynamic_cast(next_base)) { + auto bac = dynamic_cast(next_base); + total_offset += bac->offset; + next_base = bac->base; + } + *offset_out = total_offset; + return next_base; +} +} // namespace + RegVal* MemoryOffsetConstantVal::to_reg(Env* fe) { auto re = fe->make_gpr(coerce_to_reg_type(m_ts)); - if (offset == 0) { - fe->emit_ir(re, base->to_gpr(fe)); + s64 final_offset; + auto final_base = get_constant_offset_and_base(this, &final_offset); + + if (final_offset == 0) { + fe->emit_ir(re, final_base->to_gpr(fe)); } else { - fe->emit(std::make_unique(re, int64_t(offset))); - fe->emit(std::make_unique(IntegerMathKind::ADD_64, re, base->to_gpr(fe))); + fe->emit(std::make_unique(re, int64_t(final_offset))); + fe->emit(std::make_unique(IntegerMathKind::ADD_64, re, final_base->to_gpr(fe))); } return re; @@ -129,35 +149,31 @@ RegVal* MemoryOffsetVal::to_reg(Env* fe) { } RegVal* MemoryDerefVal::to_reg(Env* fe) { - // todo, support better loads/stores from the stack + auto re = fe->make_gpr(coerce_to_reg_type(m_ts)); auto base_as_co = dynamic_cast(base); if (base_as_co) { - auto re = fe->make_gpr(coerce_to_reg_type(m_ts)); - fe->emit(std::make_unique(re, base_as_co->offset, - base_as_co->base->to_gpr(fe), info)); - return re; + s64 offset; + auto final_base = get_constant_offset_and_base(base_as_co, &offset); + fe->emit_ir(re, offset, final_base->to_gpr(fe), info); } else { - auto re = fe->make_gpr(coerce_to_reg_type(m_ts)); auto addr = base->to_gpr(fe); fe->emit(std::make_unique(re, 0, addr, info)); - return re; } + return re; } RegVal* MemoryDerefVal::to_fpr(Env* fe) { - // todo, support better loads/stores from the stack auto base_as_co = dynamic_cast(base); + auto re = fe->make_fpr(coerce_to_reg_type(m_ts)); if (base_as_co) { - auto re = fe->make_fpr(coerce_to_reg_type(m_ts)); - fe->emit(std::make_unique(re, base_as_co->offset, - base_as_co->base->to_gpr(fe), info)); - return re; + s64 offset; + auto final_base = get_constant_offset_and_base(base_as_co, &offset); + fe->emit_ir(re, offset, final_base->to_gpr(fe), info); } else { - auto re = fe->make_fpr(coerce_to_reg_type(m_ts)); auto addr = base->to_gpr(fe); fe->emit(std::make_unique(re, 0, addr, info)); - return re; } + return re; } RegVal* AliasVal::to_reg(Env* fe) { diff --git a/goalc/compiler/compilation/Asm.cpp b/goalc/compiler/compilation/Asm.cpp index 71796c60c2..fee74cbcfa 100644 --- a/goalc/compiler/compilation/Asm.cpp +++ b/goalc/compiler/compilation/Asm.cpp @@ -304,7 +304,6 @@ Val* Compiler::compile_asm_svf(const goos::Object& form, const goos::Object& res info.reg = RegClass::VECTOR_FLOAT; if (as_co) { // can do a clever offset here - assert(false); env->emit_ir(src, as_co->offset, as_co->base->to_gpr(env), 16, color); } else { env->emit_ir(src, 0, dest->to_gpr(env), 16, color); diff --git a/goalc/compiler/compilation/Type.cpp b/goalc/compiler/compilation/Type.cpp index c875b6b5d0..866ca794d0 100644 --- a/goalc/compiler/compilation/Type.cpp +++ b/goalc/compiler/compilation/Type.cpp @@ -498,28 +498,44 @@ Val* Compiler::compile_deref(const goos::Object& form, const goos::Object& _rest } } - auto index_value = compile_error_guard(field_obj, env)->to_gpr(env); - if (!is_integer(index_value->type())) { - throw_compiler_error(form, "Cannot use -> with {}.", field_obj.print()); + int64_t constant_index_value; + RegVal* index_value = nullptr; + + bool has_constant_idx = try_getting_constant_integer(field_obj, &constant_index_value, env); + if (!has_constant_idx) { + index_value = compile_error_guard(field_obj, env)->to_gpr(env); + if (!is_integer(index_value->type())) { + throw_compiler_error(form, "Cannot use -> with {}.", field_obj.print()); + } } if (result->type().base_type() == "inline-array") { auto di = m_ts.get_deref_info(result->type()); auto base_type = di.result_type; assert(di.can_deref); - auto offset = compile_integer(di.stride, env)->to_gpr(env); - // todo, check for integer and avoid runtime multiply - env->emit(std::make_unique(IntegerMathKind::IMUL_32, offset, index_value)); - result = fe->alloc_val(di.result_type, result, offset); + if (has_constant_idx) { + result = fe->alloc_val(di.result_type, result, + di.stride * constant_index_value); + } else { + // todo - use shifts if possible? + RegVal* offset = fe->make_gpr(TypeSpec("int")); + compile_constant_product(offset, index_value, di.stride, env); + result = fe->alloc_val(di.result_type, result, offset); + } } else if (result->type().base_type() == "pointer") { auto di = m_ts.get_deref_info(result->type()); auto base_type = di.result_type; assert(di.mem_deref); assert(di.can_deref); - auto offset = compile_integer(di.stride, env)->to_gpr(env); - // todo, check for integer and avoid runtime multiply - env->emit(std::make_unique(IntegerMathKind::IMUL_32, offset, index_value)); - auto loc = fe->alloc_val(result->type(), result, offset); + Val* loc = nullptr; + if (has_constant_idx) { + loc = fe->alloc_val(result->type(), result, + constant_index_value * di.stride); + } else { + RegVal* offset = fe->make_gpr(TypeSpec("int")); + compile_constant_product(offset, index_value, di.stride, env); + loc = fe->alloc_val(result->type(), result, offset); + } result = fe->alloc_val(di.result_type, loc, MemLoadInfo(di)); result->mark_as_settable(); } else if (result->type().base_type() == "array") { @@ -537,15 +553,22 @@ Val* Compiler::compile_deref(const goos::Object& form, const goos::Object& _rest assert(base_type == result->type().get_single_arg()); assert(di.mem_deref); assert(di.can_deref); - // the total offset is 12 + stride * idx - auto offset = compile_integer(ARRAY_DATA_OFFSET, env)->to_gpr(env); - auto stride = compile_integer(di.stride, env)->to_gpr(env); - env->emit(std::make_unique(IntegerMathKind::IMUL_32, stride, index_value)); - env->emit_ir(IntegerMathKind::ADD_64, offset, stride); - // offset now contains the total offset. + Val* loc = nullptr; + + if (has_constant_idx) { + loc = fe->alloc_val( + loc_type, result, ARRAY_DATA_OFFSET + di.stride * constant_index_value); + } else { + // the total offset is 12 + stride * idx + auto arr_off = compile_integer(ARRAY_DATA_OFFSET, env)->to_gpr(env); + RegVal* offset = fe->make_gpr(TypeSpec("int")); + compile_constant_product(offset, index_value, di.stride, env); + env->emit_ir(IntegerMathKind::ADD_64, offset, arr_off); + + // create a location to deref (so we can do address-of and get this), with pointer type + loc = fe->alloc_val(loc_type, result, offset); + } - // create a location to deref (so we can do address-of and get this), with pointer type - auto loc = fe->alloc_val(loc_type, result, offset); // and result type. result = fe->alloc_val(di.result_type, loc, MemLoadInfo(di)); // array values should be settable diff --git a/test/goalc/source_templates/with_game/test-boxed-array-index.gc b/test/goalc/source_templates/with_game/test-boxed-array-index.gc new file mode 100644 index 0000000000..927ad3bddc --- /dev/null +++ b/test/goalc/source_templates/with_game/test-boxed-array-index.gc @@ -0,0 +1,5 @@ +(let ((arr (new 'global 'boxed-array int16 12)) + (x 3)) + ;; 12 + 3 * 2 = 18 + (format #t "~D~%" (&- (&-> arr 3) arr)) + ) diff --git a/test/goalc/test_with_game.cpp b/test/goalc/test_with_game.cpp index 3f9a7419c0..59e94e4721 100644 --- a/test/goalc/test_with_game.cpp +++ b/test/goalc/test_with_game.cpp @@ -361,6 +361,10 @@ TEST_F(WithGameTests, XMMSpill) { runner.run_static_test(env, testCategory, "test-xmm-spill.gc", {"253.0000\n0\n"}); } +TEST_F(WithGameTests, BoxedArrayIndex) { + runner.run_static_test(env, testCategory, "test-boxed-array-index.gc", {"18\n0\n"}); +} + TEST(TypeConsistency, TypeConsistency) { Compiler compiler; compiler.enable_throw_on_redefines();