Skip to content

Commit a34d817

Browse files
RepiteoRepiteo
committed
Merge pull request godotengine#71564 from dalexeev/gds-optimize-for-range
GDScript: Optimize non-constant `for`-`range`
2 parents d3956ea + a13fbc6 commit a34d817

File tree

10 files changed

+267
-119
lines changed

10 files changed

+267
-119
lines changed

‎modules/gdscript/gdscript_analyzer.cpp‎

Lines changed: 24 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -2195,101 +2195,39 @@ void GDScriptAnalyzer::resolve_if(GDScriptParser::IfNode *p_if) {
21952195
}
21962196

21972197
void GDScriptAnalyzer::resolve_for(GDScriptParser::ForNode *p_for) {
2198-
bool list_resolved = false;
2199-
2200-
// Optimize constant range() call to not allocate an array.
2201-
// Use int, Vector2i, Vector3i instead, which also can be used as range iterators.
2202-
if (p_for->list && p_for->list->type == GDScriptParser::Node::CALL) {
2203-
GDScriptParser::CallNode *call = static_cast<GDScriptParser::CallNode *>(p_for->list);
2204-
GDScriptParser::Node::Type callee_type = call->get_callee_type();
2205-
if (callee_type == GDScriptParser::Node::IDENTIFIER) {
2206-
GDScriptParser::IdentifierNode *callee = static_cast<GDScriptParser::IdentifierNode *>(call->callee);
2207-
if (callee->name == "range") {
2208-
list_resolved = true;
2209-
if (call->arguments.is_empty()) {
2210-
push_error(R"*(Invalid call for "range()" function. Expected at least 1 argument, none given.)*", call->callee);
2211-
} else if (call->arguments.size() > 3) {
2212-
push_error(vformat(R"*(Invalid call for "range()" function. Expected at most 3 arguments, %d given.)*", call->arguments.size()), call->callee);
2213-
} else {
2214-
// Now we can optimize it.
2215-
bool can_reduce = true;
2216-
Vector<Variant> args;
2217-
args.resize(call->arguments.size());
2218-
for (int i = 0; i < call->arguments.size(); i++) {
2219-
GDScriptParser::ExpressionNode *argument = call->arguments[i];
2220-
reduce_expression(argument);
2221-
2222-
if (argument->is_constant) {
2223-
if (argument->reduced_value.get_type() != Variant::INT && argument->reduced_value.get_type() != Variant::FLOAT) {
2224-
can_reduce = false;
2225-
push_error(vformat(R"*(Invalid argument for "range()" call. Argument %d should be int or float but "%s" was given.)*", i + 1, Variant::get_type_name(argument->reduced_value.get_type())), argument);
2226-
}
2227-
if (can_reduce) {
2228-
args.write[i] = argument->reduced_value;
2229-
}
2230-
} else {
2231-
can_reduce = false;
2232-
GDScriptParser::DataType argument_type = argument->get_datatype();
2233-
if (argument_type.is_variant() || !argument_type.is_hard_type()) {
2234-
mark_node_unsafe(argument);
2235-
}
2236-
if (!argument_type.is_variant() && (argument_type.builtin_type != Variant::INT && argument_type.builtin_type != Variant::FLOAT)) {
2237-
if (!argument_type.is_hard_type()) {
2238-
downgrade_node_type_source(argument);
2239-
} else {
2240-
push_error(vformat(R"*(Invalid argument for "range()" call. Argument %d should be int or float but "%s" was given.)*", i + 1, argument_type.to_string()), argument);
2241-
}
2242-
}
2243-
}
2244-
}
2198+
GDScriptParser::DataType variable_type;
2199+
GDScriptParser::DataType list_type;
22452200

2246-
Variant reduced;
2201+
if (p_for->list) {
2202+
resolve_node(p_for->list, false);
22472203

2248-
if (can_reduce) {
2249-
switch (args.size()) {
2250-
case 1:
2251-
reduced = (int32_t)args[0];
2252-
break;
2253-
case 2:
2254-
reduced = Vector2i(args[0], args[1]);
2255-
break;
2256-
case 3:
2257-
reduced = Vector3i(args[0], args[1], args[2]);
2258-
break;
2259-
}
2260-
p_for->list->is_constant = true;
2261-
p_for->list->reduced_value = reduced;
2204+
bool is_range = false;
2205+
if (p_for->list->type == GDScriptParser::Node::CALL) {
2206+
GDScriptParser::CallNode *call = static_cast<GDScriptParser::CallNode *>(p_for->list);
2207+
if (call->get_callee_type() == GDScriptParser::Node::IDENTIFIER) {
2208+
if (static_cast<GDScriptParser::IdentifierNode *>(call->callee)->name == "range") {
2209+
if (call->arguments.is_empty()) {
2210+
push_error(R"*(Invalid call for "range()" function. Expected at least 1 argument, none given.)*", call);
2211+
} else if (call->arguments.size() > 3) {
2212+
push_error(vformat(R"*(Invalid call for "range()" function. Expected at most 3 arguments, %d given.)*", call->arguments.size()), call);
22622213
}
2263-
}
2264-
2265-
if (p_for->list->is_constant) {
2266-
p_for->list->set_datatype(type_from_variant(p_for->list->reduced_value, p_for->list));
2267-
} else {
2268-
GDScriptParser::DataType list_type;
2269-
list_type.type_source = GDScriptParser::DataType::ANNOTATED_EXPLICIT;
2270-
list_type.kind = GDScriptParser::DataType::BUILTIN;
2271-
list_type.builtin_type = Variant::ARRAY;
2272-
p_for->list->set_datatype(list_type);
2214+
is_range = true;
2215+
variable_type.type_source = GDScriptParser::DataType::ANNOTATED_INFERRED;
2216+
variable_type.kind = GDScriptParser::DataType::BUILTIN;
2217+
variable_type.builtin_type = Variant::INT;
22732218
}
22742219
}
22752220
}
2276-
}
22772221

2278-
GDScriptParser::DataType variable_type;
2279-
String list_visible_type = "<unresolved type>";
2280-
if (list_resolved) {
2281-
variable_type.type_source = GDScriptParser::DataType::ANNOTATED_INFERRED;
2282-
variable_type.kind = GDScriptParser::DataType::BUILTIN;
2283-
variable_type.builtin_type = Variant::INT;
2284-
list_visible_type = "Array[int]"; // NOTE: `range()` has `Array` return type.
2285-
} else if (p_for->list) {
2286-
resolve_node(p_for->list, false);
2287-
GDScriptParser::DataType list_type = p_for->list->get_datatype();
2288-
list_visible_type = list_type.to_string();
2222+
list_type = p_for->list->get_datatype();
2223+
22892224
if (!list_type.is_hard_type()) {
22902225
mark_node_unsafe(p_for->list);
22912226
}
2292-
if (list_type.is_variant()) {
2227+
2228+
if (is_range) {
2229+
// Already solved.
2230+
} else if (list_type.is_variant()) {
22932231
variable_type.kind = GDScriptParser::DataType::VARIANT;
22942232
mark_node_unsafe(p_for->list);
22952233
} else if (list_type.has_container_element_type(0)) {
@@ -2342,7 +2280,7 @@ void GDScriptAnalyzer::resolve_for(GDScriptParser::ForNode *p_for) {
23422280
mark_node_unsafe(p_for->variable);
23432281
p_for->use_conversion_assign = true;
23442282
} else {
2345-
push_error(vformat(R"(Unable to iterate on value of type "%s" with variable of type "%s".)", list_visible_type, specified_type.to_string()), p_for->datatype_specifier);
2283+
push_error(vformat(R"(Unable to iterate on value of type "%s" with variable of type "%s".)", list_type.to_string(), specified_type.to_string()), p_for->datatype_specifier);
23462284
}
23472285
} else if (!is_type_compatible(specified_type, variable_type)) {
23482286
p_for->use_conversion_assign = true;

‎modules/gdscript/gdscript_byte_codegen.cpp‎

Lines changed: 65 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1542,16 +1542,35 @@ void GDScriptByteCodeGenerator::write_end_jump_if_shared() {
15421542
if_jmp_addrs.pop_back();
15431543
}
15441544

1545-
void GDScriptByteCodeGenerator::start_for(const GDScriptDataType &p_iterator_type, const GDScriptDataType &p_list_type) {
1545+
void GDScriptByteCodeGenerator::start_for(const GDScriptDataType &p_iterator_type, const GDScriptDataType &p_list_type, bool p_is_range) {
15461546
Address counter(Address::LOCAL_VARIABLE, add_local("@counter_pos", p_iterator_type), p_iterator_type);
1547-
Address container(Address::LOCAL_VARIABLE, add_local("@container_pos", p_list_type), p_list_type);
15481547

15491548
// Store state.
15501549
for_counter_variables.push_back(counter);
1551-
for_container_variables.push_back(container);
1550+
1551+
if (p_is_range) {
1552+
GDScriptDataType int_type;
1553+
int_type.has_type = true;
1554+
int_type.kind = GDScriptDataType::BUILTIN;
1555+
int_type.builtin_type = Variant::INT;
1556+
1557+
Address range_from(Address::LOCAL_VARIABLE, add_local("@range_from", int_type), int_type);
1558+
Address range_to(Address::LOCAL_VARIABLE, add_local("@range_to", int_type), int_type);
1559+
Address range_step(Address::LOCAL_VARIABLE, add_local("@range_step", int_type), int_type);
1560+
1561+
// Store state.
1562+
for_range_from_variables.push_back(range_from);
1563+
for_range_to_variables.push_back(range_to);
1564+
for_range_step_variables.push_back(range_step);
1565+
} else {
1566+
Address container(Address::LOCAL_VARIABLE, add_local("@container_pos", p_list_type), p_list_type);
1567+
1568+
// Store state.
1569+
for_container_variables.push_back(container);
1570+
}
15521571
}
15531572

1554-
void GDScriptByteCodeGenerator::write_for_assignment(const Address &p_list) {
1573+
void GDScriptByteCodeGenerator::write_for_list_assignment(const Address &p_list) {
15551574
const Address &container = for_container_variables.back()->get();
15561575

15571576
// Assign container.
@@ -1560,16 +1579,33 @@ void GDScriptByteCodeGenerator::write_for_assignment(const Address &p_list) {
15601579
append(p_list);
15611580
}
15621581

1563-
void GDScriptByteCodeGenerator::write_for(const Address &p_variable, bool p_use_conversion) {
1582+
void GDScriptByteCodeGenerator::write_for_range_assignment(const Address &p_from, const Address &p_to, const Address &p_step) {
1583+
const Address &range_from = for_range_from_variables.back()->get();
1584+
const Address &range_to = for_range_to_variables.back()->get();
1585+
const Address &range_step = for_range_step_variables.back()->get();
1586+
1587+
// Assign range args.
1588+
write_assign(range_from, p_from);
1589+
write_assign(range_to, p_to);
1590+
write_assign(range_step, p_step);
1591+
}
1592+
1593+
void GDScriptByteCodeGenerator::write_for(const Address &p_variable, bool p_use_conversion, bool p_is_range) {
15641594
const Address &counter = for_counter_variables.back()->get();
1565-
const Address &container = for_container_variables.back()->get();
1595+
const Address &container = p_is_range ? Address() : for_container_variables.back()->get();
1596+
const Address &range_from = p_is_range ? for_range_from_variables.back()->get() : Address();
1597+
const Address &range_to = p_is_range ? for_range_to_variables.back()->get() : Address();
1598+
const Address &range_step = p_is_range ? for_range_step_variables.back()->get() : Address();
15661599

15671600
current_breaks_to_patch.push_back(List<int>());
15681601

15691602
GDScriptFunction::Opcode begin_opcode = GDScriptFunction::OPCODE_ITERATE_BEGIN;
15701603
GDScriptFunction::Opcode iterate_opcode = GDScriptFunction::OPCODE_ITERATE;
15711604

1572-
if (container.type.has_type) {
1605+
if (p_is_range) {
1606+
begin_opcode = GDScriptFunction::OPCODE_ITERATE_BEGIN_RANGE;
1607+
iterate_opcode = GDScriptFunction::OPCODE_ITERATE_RANGE;
1608+
} else if (container.type.has_type) {
15731609
if (container.type.kind == GDScriptDataType::BUILTIN) {
15741610
switch (container.type.builtin_type) {
15751611
case Variant::INT:
@@ -1665,19 +1701,30 @@ void GDScriptByteCodeGenerator::write_for(const Address &p_variable, bool p_use_
16651701
// Begin loop.
16661702
append_opcode(begin_opcode);
16671703
append(counter);
1668-
append(container);
1704+
if (p_is_range) {
1705+
append(range_from);
1706+
append(range_to);
1707+
append(range_step);
1708+
} else {
1709+
append(container);
1710+
}
16691711
append(p_use_conversion ? temp : p_variable);
16701712
for_jmp_addrs.push_back(opcodes.size());
16711713
append(0); // End of loop address, will be patched.
16721714
append_opcode(GDScriptFunction::OPCODE_JUMP);
1673-
append(opcodes.size() + 6); // Skip over 'continue' code.
1715+
append(opcodes.size() + (p_is_range ? 7 : 6)); // Skip over 'continue' code.
16741716

16751717
// Next iteration.
16761718
int continue_addr = opcodes.size();
16771719
continue_addrs.push_back(continue_addr);
16781720
append_opcode(iterate_opcode);
16791721
append(counter);
1680-
append(container);
1722+
if (p_is_range) {
1723+
append(range_to);
1724+
append(range_step);
1725+
} else {
1726+
append(container);
1727+
}
16811728
append(p_use_conversion ? temp : p_variable);
16821729
for_jmp_addrs.push_back(opcodes.size());
16831730
append(0); // Jump destination, will be patched.
@@ -1690,7 +1737,7 @@ void GDScriptByteCodeGenerator::write_for(const Address &p_variable, bool p_use_
16901737
}
16911738
}
16921739

1693-
void GDScriptByteCodeGenerator::write_endfor() {
1740+
void GDScriptByteCodeGenerator::write_endfor(bool p_is_range) {
16941741
// Jump back to loop check.
16951742
append_opcode(GDScriptFunction::OPCODE_JUMP);
16961743
append(continue_addrs.back()->get());
@@ -1710,7 +1757,13 @@ void GDScriptByteCodeGenerator::write_endfor() {
17101757

17111758
// Pop state.
17121759
for_counter_variables.pop_back();
1713-
for_container_variables.pop_back();
1760+
if (p_is_range) {
1761+
for_range_from_variables.pop_back();
1762+
for_range_to_variables.pop_back();
1763+
for_range_step_variables.pop_back();
1764+
} else {
1765+
for_container_variables.pop_back();
1766+
}
17141767
}
17151768

17161769
void GDScriptByteCodeGenerator::start_while_condition() {

‎modules/gdscript/gdscript_byte_codegen.h‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,9 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
144144
List<int> for_jmp_addrs;
145145
List<Address> for_counter_variables;
146146
List<Address> for_container_variables;
147+
List<Address> for_range_from_variables;
148+
List<Address> for_range_to_variables;
149+
List<Address> for_range_step_variables;
147150
List<int> while_jmp_addrs;
148151
List<int> continue_addrs;
149152

@@ -535,10 +538,11 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
535538
virtual void write_endif() override;
536539
virtual void write_jump_if_shared(const Address &p_value) override;
537540
virtual void write_end_jump_if_shared() override;
538-
virtual void start_for(const GDScriptDataType &p_iterator_type, const GDScriptDataType &p_list_type) override;
539-
virtual void write_for_assignment(const Address &p_list) override;
540-
virtual void write_for(const Address &p_variable, bool p_use_conversion) override;
541-
virtual void write_endfor() override;
541+
virtual void start_for(const GDScriptDataType &p_iterator_type, const GDScriptDataType &p_list_type, bool p_is_range) override;
542+
virtual void write_for_list_assignment(const Address &p_list) override;
543+
virtual void write_for_range_assignment(const Address &p_from, const Address &p_to, const Address &p_step) override;
544+
virtual void write_for(const Address &p_variable, bool p_use_conversion, bool p_is_range) override;
545+
virtual void write_endfor(bool p_is_range) override;
542546
virtual void start_while_condition() override;
543547
virtual void write_while(const Address &p_condition) override;
544548
virtual void write_endwhile() override;

‎modules/gdscript/gdscript_codegen.h‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,11 @@ class GDScriptCodeGenerator {
148148
virtual void write_endif() = 0;
149149
virtual void write_jump_if_shared(const Address &p_value) = 0;
150150
virtual void write_end_jump_if_shared() = 0;
151-
virtual void start_for(const GDScriptDataType &p_iterator_type, const GDScriptDataType &p_list_type) = 0;
152-
virtual void write_for_assignment(const Address &p_list) = 0;
153-
virtual void write_for(const Address &p_variable, bool p_use_conversion) = 0;
154-
virtual void write_endfor() = 0;
151+
virtual void start_for(const GDScriptDataType &p_iterator_type, const GDScriptDataType &p_list_type, bool p_is_range) = 0;
152+
virtual void write_for_list_assignment(const Address &p_list) = 0;
153+
virtual void write_for_range_assignment(const Address &p_from, const Address &p_to, const Address &p_step) = 0;
154+
virtual void write_for(const Address &p_variable, bool p_use_conversion, bool p_is_range) = 0;
155+
virtual void write_endfor(bool p_is_range) = 0;
155156
virtual void start_while_condition() = 0; // Used to allow a jump to the expression evaluation.
156157
virtual void write_while(const Address &p_condition) = 0;
157158
virtual void write_endwhile() = 0;

‎modules/gdscript/gdscript_compiler.cpp‎

Lines changed: 54 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,20 +2045,64 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui
20452045

20462046
GDScriptCodeGenerator::Address iterator = codegen.add_local(for_n->variable->name, _gdtype_from_datatype(for_n->variable->get_datatype(), codegen.script));
20472047

2048-
gen->start_for(iterator.type, _gdtype_from_datatype(for_n->list->get_datatype(), codegen.script));
2049-
2050-
GDScriptCodeGenerator::Address list = _parse_expression(codegen, err, for_n->list);
2051-
if (err) {
2052-
return err;
2048+
// Optimize `range()` call to not allocate an array.
2049+
GDScriptParser::CallNode *range_call = nullptr;
2050+
if (for_n->list && for_n->list->type == GDScriptParser::Node::CALL) {
2051+
GDScriptParser::CallNode *call = static_cast<GDScriptParser::CallNode *>(for_n->list);
2052+
if (call->get_callee_type() == GDScriptParser::Node::IDENTIFIER) {
2053+
if (static_cast<GDScriptParser::IdentifierNode *>(call->callee)->name == "range") {
2054+
range_call = call;
2055+
}
2056+
}
20532057
}
20542058

2055-
gen->write_for_assignment(list);
2059+
gen->start_for(iterator.type, _gdtype_from_datatype(for_n->list->get_datatype(), codegen.script), range_call != nullptr);
20562060

2057-
if (list.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
2058-
codegen.generator->pop_temporary();
2061+
if (range_call != nullptr) {
2062+
Vector<GDScriptCodeGenerator::Address> args;
2063+
args.resize(range_call->arguments.size());
2064+
2065+
for (int j = 0; j < args.size(); j++) {
2066+
args.write[j] = _parse_expression(codegen, err, range_call->arguments[j]);
2067+
if (err) {
2068+
return err;
2069+
}
2070+
}
2071+
2072+
switch (args.size()) {
2073+
case 1:
2074+
gen->write_for_range_assignment(codegen.add_constant(0), args[0], codegen.add_constant(1));
2075+
break;
2076+
case 2:
2077+
gen->write_for_range_assignment(args[0], args[1], codegen.add_constant(1));
2078+
break;
2079+
case 3:
2080+
gen->write_for_range_assignment(args[0], args[1], args[2]);
2081+
break;
2082+
default:
2083+
_set_error(R"*(Analyzer bug: Wrong "range()" argument count.)*", range_call);
2084+
return ERR_BUG;
2085+
}
2086+
2087+
for (int j = 0; j < args.size(); j++) {
2088+
if (args[j].mode == GDScriptCodeGenerator::Address::TEMPORARY) {
2089+
codegen.generator->pop_temporary();
2090+
}
2091+
}
2092+
} else {
2093+
GDScriptCodeGenerator::Address list = _parse_expression(codegen, err, for_n->list);
2094+
if (err) {
2095+
return err;
2096+
}
2097+
2098+
gen->write_for_list_assignment(list);
2099+
2100+
if (list.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
2101+
codegen.generator->pop_temporary();
2102+
}
20592103
}
20602104

2061-
gen->write_for(iterator, for_n->use_conversion_assign);
2105+
gen->write_for(iterator, for_n->use_conversion_assign, range_call != nullptr);
20622106

20632107
// Loop variables must be cleared even when `break`/`continue` is used.
20642108
List<GDScriptCodeGenerator::Address> loop_locals = _add_block_locals(codegen, for_n->loop);
@@ -2070,7 +2114,7 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui
20702114
return err;
20712115
}
20722116

2073-
gen->write_endfor();
2117+
gen->write_endfor(range_call != nullptr);
20742118

20752119
_clear_block_locals(codegen, loop_locals); // Outside loop, after block - for `break` and normal exit.
20762120

0 commit comments

Comments
 (0)