authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2022-03-04 17:19:36-07:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2022-03-04 18:27:46-07:00
logf2a5d0bf94897554e25e889dc1c6c4c7fc6c1217
tree0847593f0683a3605f72b2ada866b56c1eb1357c
parentd3648cc0305ee94e24dd4378376e204587bc6f51

stage2: fix tuple assigned to variable

Before this we would see ZIR code like this: ``` %69 = alloc_inferred_mut() %70 = array_base_ptr(%69) %71 = elem_ptr_imm(%70, 0) ``` This would crash the compiler because it expects to see a `coerce_result_ptr` instruction after `alloc_inferred_mut`, but that does not happen in this case because there is no type to coerce the result pointer to. In this commit I modified AstGen so that it has similar codegen as when using a const instead of a var: ``` %69 = alloc_inferred_mut() %76 = array_init_anon(.{%71, %73, %75}) %77 = store_to_inferred_ptr(%69, %76) ``` This does not obey result locations, meaning if you call a function inside the initializer, it will end up doing a copy into the LHS. Solving this problem, or changing the language to make this legal, will be left for my future self to deal with. Hi future self! I see you reading this commit log. Hope you're doing OK buddy. Sema for `store_ptr` of a tuple where the pointer is in fact the same element type as the operand had an issue where the comptime fields would get incorrectly lowered to runtime stores to bogus addresses. This is solved with an exception to the optimization in Sema for storing pointers that handles tuples element-wise. In the case that we are storing a tuple to itself, it skips the optimization. This results in better code and avoids the problem. However this caused a regression in GeneralPurposeAllocator from the standard library. I regressed the test runner code back to the simpler path. It's too hard to debug standard library code in the LLVM backend right now since we don't have debug info hooked up. Also, we didn't have any behavior test coverage of whatever was regressed, so let's try to get that coverage added as a stepping stone to getting the standard library working.

4 files changed, 23 insertions(+), 11 deletions(-)

lib/std/special/test_runner.zig+3-4
...@@ -23,9 +23,7 @@ fn processArgs() void {...@@ -23,9 +23,7 @@ fn processArgs() void {
23}23}
2424
25pub fn main() void {25pub fn main() void {
26 if (builtin.zig_backend != .stage1 and26 if (builtin.zig_backend != .stage1) {
27 builtin.zig_backend != .stage2_llvm)
28 {
29 return main2() catch @panic("test failure");27 return main2() catch @panic("test failure");
30 }28 }
31 if (builtin.zig_backend == .stage1) processArgs();29 if (builtin.zig_backend == .stage1) processArgs();
...@@ -144,7 +142,8 @@ pub fn main2() anyerror!void {...@@ -144,7 +142,8 @@ pub fn main2() anyerror!void {
144 };142 };
145 }143 }
146 if (builtin.zig_backend == .stage2_wasm or144 if (builtin.zig_backend == .stage2_wasm or
147 builtin.zig_backend == .stage2_x86_64)145 builtin.zig_backend == .stage2_x86_64 or
146 builtin.zig_backend == .stage2_llvm)
148 {147 {
149 const passed = builtin.test_functions.len - skipped - failed;148 const passed = builtin.test_functions.len - skipped - failed;
150 const stderr = std.io.getStdErr();149 const stderr = std.io.getStdErr();
src/AstGen.zig+11-1
...@@ -1340,9 +1340,19 @@ fn arrayInitExpr(...@@ -1340,9 +1340,19 @@ fn arrayInitExpr(
1340 return arrayInitExprRlTy(gz, scope, node, array_init.ast.elements, elem_type, .array_init);1340 return arrayInitExprRlTy(gz, scope, node, array_init.ast.elements, elem_type, .array_init);
1341 }1341 }
1342 },1342 },
1343 .ptr, .inferred_ptr => |ptr_inst| {1343 .ptr => |ptr_inst| {
1344 return arrayInitExprRlPtr(gz, scope, rl, node, ptr_inst, array_init.ast.elements, types.array);1344 return arrayInitExprRlPtr(gz, scope, rl, node, ptr_inst, array_init.ast.elements, types.array);
1345 },1345 },
1346 .inferred_ptr => |ptr_inst| {
1347 if (types.array == .none) {
1348 // We treat this case differently so that we don't get a crash when
1349 // analyzing array_base_ptr against an alloc_inferred_mut.
1350 const result = try arrayInitExprRlNone(gz, scope, node, array_init.ast.elements, .array_init_anon);
1351 return rvalue(gz, rl, result, node);
1352 } else {
1353 return arrayInitExprRlPtr(gz, scope, rl, node, ptr_inst, array_init.ast.elements, types.array);
1354 }
1355 },
1346 .block_ptr => |block_gz| {1356 .block_ptr => |block_gz| {
1347 return arrayInitExprRlPtr(gz, scope, rl, node, block_gz.rl_ptr, array_init.ast.elements, types.array);1357 return arrayInitExprRlPtr(gz, scope, rl, node, block_gz.rl_ptr, array_init.ast.elements, types.array);
1348 },1358 },
src/Sema.zig+6-4
...@@ -16458,11 +16458,13 @@ fn storePtr2(...@@ -16458,11 +16458,13 @@ fn storePtr2(
16458 // To generate better code for tuples, we detect a tuple operand here, and16458 // To generate better code for tuples, we detect a tuple operand here, and
16459 // analyze field loads and stores directly. This avoids an extra allocation + memcpy16459 // analyze field loads and stores directly. This avoids an extra allocation + memcpy
16460 // which would occur if we used `coerce`.16460 // which would occur if we used `coerce`.
16461 // However, we avoid this mechanism if the destination element type is
16462 // the same tuple as the source, because the regular store will be better for this case.
16461 const operand_ty = sema.typeOf(uncasted_operand);16463 const operand_ty = sema.typeOf(uncasted_operand);
16462 if (operand_ty.castTag(.tuple)) |payload| {16464 if (operand_ty.isTuple() and !elem_ty.eql(operand_ty)) {
16463 const tuple_fields_len = payload.data.types.len;16465 const tuple = operand_ty.tupleFields();
16464 var i: u32 = 0;16466 for (tuple.types) |_, i_usize| {
16465 while (i < tuple_fields_len) : (i += 1) {16467 const i = @intCast(u32, i_usize);
16466 const elem_src = operand_src; // TODO better source location16468 const elem_src = operand_src; // TODO better source location
16467 const elem = try tupleField(sema, block, uncasted_operand, i, operand_src, elem_src);16469 const elem = try tupleField(sema, block, uncasted_operand, i, operand_src, elem_src);
16468 const elem_index = try sema.addIntUnsigned(Type.usize, i);16470 const elem_index = try sema.addIntUnsigned(Type.usize, i);
test/behavior/struct.zig+3-2
...@@ -976,12 +976,13 @@ test "fully anonymous list literal" {...@@ -976,12 +976,13 @@ test "fully anonymous list literal" {
976}976}
977977
978test "tuple assigned to variable" {978test "tuple assigned to variable" {
979 if (builtin.zig_backend != .stage1) return error.SkipZigTest; // TODO
980
981 var vec = .{ @as(i32, 22), @as(i32, 55), @as(i32, 99) };979 var vec = .{ @as(i32, 22), @as(i32, 55), @as(i32, 99) };
982 try expect(vec.@"0" == 22);980 try expect(vec.@"0" == 22);
983 try expect(vec.@"1" == 55);981 try expect(vec.@"1" == 55);
984 try expect(vec.@"2" == 99);982 try expect(vec.@"2" == 99);
983 try expect(vec[0] == 22);
984 try expect(vec[1] == 55);
985 try expect(vec[2] == 99);
985}986}
986987
987test "comptime struct field" {988test "comptime struct field" {