Commit 63788b2a511eb87974065a052e2436b0c6202544 | zig

author	Veikka Tuominen <git@vexu.eu> 2022-02-24 23:45:49+02:00
committer	Andrew Kelley <andrew@ziglang.org> 2022-02-24 18:32:08-05:00
log	63788b2a511eb87974065a052e2436b0c6202544
tree	ef11ed08dc3b7328ba5de9c778a1afe9e0d48744
parent	52a2aa11e2768a5d7881377ddb7a053d81e4cff4

stage2: change how stale `store_to_block_ptr`s are detected

Instead of explicitly setting lhs to .none,
check if the lhs instruction was analyzed.
This simpler approach also handles stores from nested blocks correctly.

4 files changed, 19 insertions(+), 32 deletions(-)

src
- AstGen.zig
- Sema.zig
test
- behavior.zig
- behavior
  - bugs
    10970.zig

src/AstGen.zig-27

 @@ -1760,19 +1760,6 @@ fn breakExpr(parent_gz: *GenZir, parent_scope: *Scope, node: Ast.Node.Index) Inn
                     .block_ptr => {
                         const br = try parent_gz.addBreak(break_tag, block_inst, operand);
                         try block_gz.labeled_breaks.append(astgen.gpa, br);
-                        // if list grew as much as rvalue_rl_count, then a break
-                        // inside operand already saved the store_to_block_ptr
-                        const have_store_to_block = block_gz.rvalue_rl_count >
-                            block_gz.labeled_store_to_block_ptr_list.items.len;
-                        if (have_store_to_block) {
-                            const zir_tags = parent_gz.astgen.instructions.items(.tag);
-                            const zir_datas = parent_gz.astgen.instructions.items(.data);
-                            const store_inst = @intCast(u32, zir_tags.len - 2);
-                            assert(zir_tags[store_inst] == .store_to_block_ptr);
-                            assert(zir_datas[store_inst].bin.lhs == block_gz.rl_ptr);
-                            try block_gz.labeled_store_to_block_ptr_list.append(astgen.gpa, store_inst);
-                        }
                     },
                     .ptr => {
                         // In this case we don't have any mechanism to intercept it;
 @@ -1955,7 +1942,6 @@ fn labeledBlockExpr(
     block_scope.setBreakResultLoc(rl);
     defer block_scope.unstack();
     defer block_scope.labeled_breaks.deinit(astgen.gpa);
-    defer block_scope.labeled_store_to_block_ptr_list.deinit(astgen.gpa);
     try blockExprStmts(&block_scope, &block_scope.base, statements);
 @@ -1963,9 +1949,7 @@ fn labeledBlockExpr(
         try astgen.appendErrorTok(label_token, "unused block label", .{});
     }
-    const zir_tags = gz.astgen.instructions.items(.tag);
     const zir_datas = gz.astgen.instructions.items(.data);
     const strat = rl.strategy(&block_scope);
     switch (strat.tag) {
         .break_void => {
 @@ -1981,11 +1965,6 @@ fn labeledBlockExpr(
         .break_operand => {
             // All break operands are values that did not use the result location pointer.
             if (strat.elide_store_to_block_ptr_instructions) {
-                for (block_scope.labeled_store_to_block_ptr_list.items) |inst| {
-                    // Mark as elided for removal below.
-                    assert(zir_tags[inst] == .store_to_block_ptr);
-                    zir_datas[inst].bin.lhs = .none;
-                }
                 try block_scope.setBlockBodyEliding(block_inst);
             } else {
                 try block_scope.setBlockBody(block_inst);
 @@ -5322,7 +5301,6 @@ fn whileExpr(
     loop_scope.setBreakResultLoc(rl);
     defer loop_scope.unstack();
     defer loop_scope.labeled_breaks.deinit(astgen.gpa);
-    defer loop_scope.labeled_store_to_block_ptr_list.deinit(astgen.gpa);
     var continue_scope = parent_gz.makeSubBlock(&loop_scope.base);
     defer continue_scope.unstack();
 @@ -5573,7 +5551,6 @@ fn forExpr(
     loop_scope.setBreakResultLoc(rl);
     defer loop_scope.unstack();
     defer loop_scope.labeled_breaks.deinit(astgen.gpa);
-    defer loop_scope.labeled_store_to_block_ptr_list.deinit(astgen.gpa);
     var cond_scope = parent_gz.makeSubBlock(&loop_scope.base);
     defer cond_scope.unstack();
 @@ -9553,10 +9530,6 @@ const GenZir = struct {
     /// Tracks `break :foo bar` instructions so they can possibly be elided later if
     /// the labeled block ends up not needing a result location pointer.
     labeled_breaks: ArrayListUnmanaged(Zir.Inst.Index) = .{},
-    /// Tracks `store_to_block_ptr` instructions that correspond to break instructions
-    /// so they can possibly be elided later if the labeled block ends up not needing
-    /// a result location pointer.
-    labeled_store_to_block_ptr_list: ArrayListUnmanaged(Zir.Inst.Index) = .{},
     suspend_node: Ast.Node.Index = 0,
     nosuspend_node: Ast.Node.Index = 0,

src/Sema.zig+3-5

 @@ -3247,12 +3247,10 @@ fn zirStoreToBlockPtr(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileE
     defer tracy.end();
     const bin_inst = sema.code.instructions.items(.data)[inst].bin;
-    if (bin_inst.lhs == .none) {
+    const ptr = sema.inst_map.get(@enumToInt(bin_inst.lhs) - @as(u32, Zir.Inst.Ref.typed_value_map.len)) orelse {
-        // This is an elided instruction, but AstGen was not smart enough
+        // This is an elided instruction, but AstGen was unable to omit it.
-        // to omit it.
         return;
-    }
+    };
-    const ptr = sema.resolveInst(bin_inst.lhs);
     const value = sema.resolveInst(bin_inst.rhs);
     const ptr_ty = try Type.ptr(sema.arena, .{
         .pointee_type = sema.typeOf(value),

test/behavior.zig+1

...	@@ -103,6 +103,7 @@ test {	...	@@ -103,6 +103,7 @@ test {
103	{	103	{
104	// Tests that pass for stage1, llvm backend, C backend	104	// Tests that pass for stage1, llvm backend, C backend
105	_ = @import("behavior/bugs/9584.zig");	105	_ = @import("behavior/bugs/9584.zig");
		106	_ = @import("behavior/bugs/10970.zig");
106	_ = @import("behavior/cast_int.zig");	107	_ = @import("behavior/cast_int.zig");
107	_ = @import("behavior/eval.zig");	108	_ = @import("behavior/eval.zig");
108	_ = @import("behavior/int128.zig");	109	_ = @import("behavior/int128.zig");

test/behavior/bugs/10970.zig created+15

...	@@ -0,0 +1,15 @@
1	const builtin = @import("builtin");
2
3	fn retOpt() ?u32 {
4	return null;
5	}
6	test {
7	var cond = true;
8	const opt = while (cond) {
9	if (retOpt()) \|opt\| {
10	break opt;
11	}
12	break 1;
13	} else 2;
14	_ = opt;
15	}