authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2018-03-09 14:20:44-05:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2018-03-09 14:20:44-05:00
log6db9be8900bf43632c8a98d91c6a92f30b33500a
treee5a9dde7f5cc3a44439e22c16695edac010b55ab
parentaaf2230ae89d74497042b7fada8c8023bf274dbd

don't memoize comptime functions if they can mutate state via parameters

closes #639

6 files changed, 77 insertions(+), 9 deletions(-)

src/all_types.hpp+8-1
...@@ -1168,10 +1168,17 @@ struct TypeTableEntry {...@@ -1168,10 +1168,17 @@ struct TypeTableEntry {
1168 LLVMTypeRef type_ref;1168 LLVMTypeRef type_ref;
1169 ZigLLVMDIType *di_type;1169 ZigLLVMDIType *di_type;
11701170
1171 bool zero_bits;1171 bool zero_bits; // this is denormalized data
1172 bool is_copyable;1172 bool is_copyable;
1173 bool gen_h_loop_flag;1173 bool gen_h_loop_flag;
11741174
1175 // This is denormalized data. The simplest type that has this
1176 // flag set to true is a mutable pointer. A const pointer has
1177 // the same value for this flag as the child type.
1178 // If a struct has any fields that have this flag true, then
1179 // the flag is true for the struct.
1180 bool can_mutate_state_through_it;
1181
1175 union {1182 union {
1176 TypeTableEntryPointer pointer;1183 TypeTableEntryPointer pointer;
1177 TypeTableEntryInt integral;1184 TypeTableEntryInt integral;
src/analyze.cpp+28
...@@ -398,6 +398,7 @@ TypeTableEntry *get_pointer_to_type_extra(CodeGen *g, TypeTableEntry *child_type...@@ -398,6 +398,7 @@ TypeTableEntry *get_pointer_to_type_extra(CodeGen *g, TypeTableEntry *child_type
398398
399 TypeTableEntry *entry = new_type_table_entry(TypeTableEntryIdPointer);399 TypeTableEntry *entry = new_type_table_entry(TypeTableEntryIdPointer);
400 entry->is_copyable = true;400 entry->is_copyable = true;
401 entry->can_mutate_state_through_it = is_const ? child_type->can_mutate_state_through_it : true;
401402
402 const char *const_str = is_const ? "const " : "";403 const char *const_str = is_const ? "const " : "";
403 const char *volatile_str = is_volatile ? "volatile " : "";404 const char *volatile_str = is_volatile ? "volatile " : "";
...@@ -482,6 +483,7 @@ TypeTableEntry *get_maybe_type(CodeGen *g, TypeTableEntry *child_type) {...@@ -482,6 +483,7 @@ TypeTableEntry *get_maybe_type(CodeGen *g, TypeTableEntry *child_type) {
482 assert(child_type->type_ref || child_type->zero_bits);483 assert(child_type->type_ref || child_type->zero_bits);
483 assert(child_type->di_type);484 assert(child_type->di_type);
484 entry->is_copyable = type_is_copyable(g, child_type);485 entry->is_copyable = type_is_copyable(g, child_type);
486 entry->can_mutate_state_through_it = child_type->can_mutate_state_through_it;
485487
486 buf_resize(&entry->name, 0);488 buf_resize(&entry->name, 0);
487 buf_appendf(&entry->name, "?%s", buf_ptr(&child_type->name));489 buf_appendf(&entry->name, "?%s", buf_ptr(&child_type->name));
...@@ -572,6 +574,7 @@ TypeTableEntry *get_error_union_type(CodeGen *g, TypeTableEntry *err_set_type, T...@@ -572,6 +574,7 @@ TypeTableEntry *get_error_union_type(CodeGen *g, TypeTableEntry *err_set_type, T
572 entry->is_copyable = true;574 entry->is_copyable = true;
573 assert(payload_type->di_type);575 assert(payload_type->di_type);
574 ensure_complete_type(g, payload_type);576 ensure_complete_type(g, payload_type);
577 entry->can_mutate_state_through_it = payload_type->can_mutate_state_through_it;
575578
576 buf_resize(&entry->name, 0);579 buf_resize(&entry->name, 0);
577 buf_appendf(&entry->name, "%s!%s", buf_ptr(&err_set_type->name), buf_ptr(&payload_type->name));580 buf_appendf(&entry->name, "%s!%s", buf_ptr(&err_set_type->name), buf_ptr(&payload_type->name));
...@@ -730,6 +733,7 @@ TypeTableEntry *get_slice_type(CodeGen *g, TypeTableEntry *ptr_type) {...@@ -730,6 +733,7 @@ TypeTableEntry *get_slice_type(CodeGen *g, TypeTableEntry *ptr_type) {
730733
731 TypeTableEntry *entry = new_type_table_entry(TypeTableEntryIdStruct);734 TypeTableEntry *entry = new_type_table_entry(TypeTableEntryIdStruct);
732 entry->is_copyable = true;735 entry->is_copyable = true;
736 entry->can_mutate_state_through_it = ptr_type->can_mutate_state_through_it;
733737
734 // replace the & with [] to go from a ptr type name to a slice type name738 // replace the & with [] to go from a ptr type name to a slice type name
735 buf_resize(&entry->name, 0);739 buf_resize(&entry->name, 0);
...@@ -1735,6 +1739,8 @@ TypeTableEntry *get_struct_type(CodeGen *g, const char *type_name, const char *f...@@ -1735,6 +1739,8 @@ TypeTableEntry *get_struct_type(CodeGen *g, const char *type_name, const char *f
1735 struct_type->data.structure.gen_field_count += 1;1739 struct_type->data.structure.gen_field_count += 1;
1736 } else {1740 } else {
1737 field->gen_index = SIZE_MAX;1741 field->gen_index = SIZE_MAX;
1742 struct_type->can_mutate_state_through_it = struct_type->can_mutate_state_through_it ||
1743 field->type_entry->can_mutate_state_through_it;
1738 }1744 }
17391745
1740 auto prev_entry = struct_type->data.structure.fields_by_name.put_unique(field->name, field);1746 auto prev_entry = struct_type->data.structure.fields_by_name.put_unique(field->name, field);
...@@ -2475,6 +2481,9 @@ static void resolve_struct_zero_bits(CodeGen *g, TypeTableEntry *struct_type) {...@@ -2475,6 +2481,9 @@ static void resolve_struct_zero_bits(CodeGen *g, TypeTableEntry *struct_type) {
2475 if (!type_has_bits(field_type))2481 if (!type_has_bits(field_type))
2476 continue;2482 continue;
24772483
2484 struct_type->can_mutate_state_through_it = struct_type->can_mutate_state_through_it ||
2485 field_type->can_mutate_state_through_it;
2486
2478 if (gen_field_index == 0) {2487 if (gen_field_index == 0) {
2479 if (struct_type->data.structure.layout == ContainerLayoutPacked) {2488 if (struct_type->data.structure.layout == ContainerLayoutPacked) {
2480 struct_type->data.structure.abi_alignment = 1;2489 struct_type->data.structure.abi_alignment = 1;
...@@ -2662,6 +2671,8 @@ static void resolve_union_zero_bits(CodeGen *g, TypeTableEntry *union_type) {...@@ -2662,6 +2671,8 @@ static void resolve_union_zero_bits(CodeGen *g, TypeTableEntry *union_type) {
2662 }2671 }
2663 }2672 }
2664 union_field->type_entry = field_type;2673 union_field->type_entry = field_type;
2674 union_type->can_mutate_state_through_it = union_type->can_mutate_state_through_it ||
2675 field_type->can_mutate_state_through_it;
26652676
2666 if (field_node->data.struct_field.value != nullptr && !decl_node->data.container_decl.auto_enum) {2677 if (field_node->data.struct_field.value != nullptr && !decl_node->data.container_decl.auto_enum) {
2667 ErrorMsg *msg = add_node_error(g, field_node->data.struct_field.value,2678 ErrorMsg *msg = add_node_error(g, field_node->data.struct_field.value,
...@@ -4565,6 +4576,23 @@ bool generic_fn_type_id_eql(GenericFnTypeId *a, GenericFnTypeId *b) {...@@ -4565,6 +4576,23 @@ bool generic_fn_type_id_eql(GenericFnTypeId *a, GenericFnTypeId *b) {
4565 return true;4576 return true;
4566}4577}
45674578
4579bool fn_eval_cacheable(Scope *scope) {
4580 while (scope) {
4581 if (scope->id == ScopeIdVarDecl) {
4582 ScopeVarDecl *var_scope = (ScopeVarDecl *)scope;
4583 if (var_scope->var->value->type->can_mutate_state_through_it)
4584 return false;
4585 } else if (scope->id == ScopeIdFnDef) {
4586 return true;
4587 } else {
4588 zig_unreachable();
4589 }
4590
4591 scope = scope->parent;
4592 }
4593 zig_unreachable();
4594}
4595
4568uint32_t fn_eval_hash(Scope* scope) {4596uint32_t fn_eval_hash(Scope* scope) {
4569 uint32_t result = 0;4597 uint32_t result = 0;
4570 while (scope) {4598 while (scope) {
src/analyze.hpp+1
...@@ -195,5 +195,6 @@ TypeTableEntry *get_auto_err_set_type(CodeGen *g, FnTableEntry *fn_entry);...@@ -195,5 +195,6 @@ TypeTableEntry *get_auto_err_set_type(CodeGen *g, FnTableEntry *fn_entry);
195195
196uint32_t get_coro_frame_align_bytes(CodeGen *g);196uint32_t get_coro_frame_align_bytes(CodeGen *g);
197bool fn_type_can_fail(FnTypeId *fn_type_id);197bool fn_type_can_fail(FnTypeId *fn_type_id);
198bool fn_eval_cacheable(Scope *scope);
198199
199#endif200#endif
src/ir.cpp+11-6
...@@ -11830,12 +11830,15 @@ static TypeTableEntry *ir_analyze_fn_call(IrAnalyze *ira, IrInstructionCall *cal...@@ -11830,12 +11830,15 @@ static TypeTableEntry *ir_analyze_fn_call(IrAnalyze *ira, IrInstructionCall *cal
11830 return_type = specified_return_type;11830 return_type = specified_return_type;
11831 }11831 }
1183211832
11833 IrInstruction *result;11833 bool cacheable = fn_eval_cacheable(exec_scope);
11834 IrInstruction *result = nullptr;
11835 if (cacheable) {
11836 auto entry = ira->codegen->memoized_fn_eval_table.maybe_get(exec_scope);
11837 if (entry)
11838 result = entry->value;
11839 }
1183411840
11835 auto entry = ira->codegen->memoized_fn_eval_table.maybe_get(exec_scope);11841 if (result == nullptr) {
11836 if (entry) {
11837 result = entry->value;
11838 } else {
11839 // Analyze the fn body block like any other constant expression.11842 // Analyze the fn body block like any other constant expression.
11840 AstNode *body_node = fn_entry->body_node;11843 AstNode *body_node = fn_entry->body_node;
11841 result = ir_eval_const_value(ira->codegen, exec_scope, body_node, return_type,11844 result = ir_eval_const_value(ira->codegen, exec_scope, body_node, return_type,
...@@ -11859,7 +11862,9 @@ static TypeTableEntry *ir_analyze_fn_call(IrAnalyze *ira, IrInstructionCall *cal...@@ -11859,7 +11862,9 @@ static TypeTableEntry *ir_analyze_fn_call(IrAnalyze *ira, IrInstructionCall *cal
11859 }11862 }
11860 }11863 }
1186111864
11862 ira->codegen->memoized_fn_eval_table.put(exec_scope, result);11865 if (cacheable) {
11866 ira->codegen->memoized_fn_eval_table.put(exec_scope, result);
11867 }
1186311868
11864 if (type_is_invalid(result->value.type))11869 if (type_is_invalid(result->value.type))
11865 return ira->codegen->builtin_types.entry_invalid;11870 return ira->codegen->builtin_types.entry_invalid;
std/sort.zig+1-2
...@@ -964,8 +964,7 @@ fn u8desc(lhs: &const u8, rhs: &const u8) bool {...@@ -964,8 +964,7 @@ fn u8desc(lhs: &const u8, rhs: &const u8) bool {
964964
965test "stable sort" {965test "stable sort" {
966 testStableSort();966 testStableSort();
967 // TODO: uncomment this after https://github.com/zig-lang/zig/issues/639967 comptime testStableSort();
968 //comptime testStableSort();
969}968}
970fn testStableSort() void {969fn testStableSort() void {
971 var expected = []IdAndValue {970 var expected = []IdAndValue {
test/cases/eval.zig+28
...@@ -420,3 +420,31 @@ test "binary math operator in partially inlined function" {...@@ -420,3 +420,31 @@ test "binary math operator in partially inlined function" {
420 assert(s[2] == 0x90a0b0c);420 assert(s[2] == 0x90a0b0c);
421 assert(s[3] == 0xd0e0f10);421 assert(s[3] == 0xd0e0f10);
422}422}
423
424
425test "comptime function with the same args is memoized" {
426 comptime {
427 assert(MakeType(i32) == MakeType(i32));
428 assert(MakeType(i32) != MakeType(f64));
429 }
430}
431
432fn MakeType(comptime T: type) type {
433 return struct {
434 field: T,
435 };
436}
437
438test "comptime function with mutable pointer is not memoized" {
439 comptime {
440 var x: i32 = 1;
441 const ptr = &x;
442 increment(ptr);
443 increment(ptr);
444 assert(x == 3);
445 }
446}
447
448fn increment(value: &i32) void {
449 *value += 1;
450}