Merge branch 'dist-foundation' into flow/sx-foundation/F2.2
# Conflicts: # issues/0078-string-eq-operand-of-short-circuit-and-invalid-phi.md
This commit is contained in:
53
examples/0045-basic-string-eq-short-circuit.sx
Normal file
53
examples/0045-basic-string-eq-short-circuit.sx
Normal file
@@ -0,0 +1,53 @@
|
||||
// String `==`/`!=` as an operand of a short-circuit `and`/`or`.
|
||||
//
|
||||
// A string compare lowers to its own multi-block memcmp sub-CFG, so the
|
||||
// operand finishes in a later basic block than the one the short-circuit
|
||||
// started in. The `and`/`or` merge PHI must take that actual block as the
|
||||
// incoming predecessor.
|
||||
//
|
||||
// Regression (issue 0078): combining string equality with `and`/`or` used to
|
||||
// emit invalid LLVM (`PHI node entries do not match predecessors!`).
|
||||
#import "modules/std.sx";
|
||||
|
||||
Json :: enum {
|
||||
str: string;
|
||||
int_: s64;
|
||||
null_;
|
||||
}
|
||||
|
||||
main :: () {
|
||||
a := "k";
|
||||
b := "v";
|
||||
|
||||
// string == on both sides of `and`
|
||||
and_tt := a == "k" and b == "v";
|
||||
and_tf := a == "k" and b == "x";
|
||||
and_ft := a == "z" and b == "v";
|
||||
print("and: {} {} {}\n", and_tt, and_tf, and_ft);
|
||||
|
||||
// string == on both sides of `or`
|
||||
or_ff := a == "z" or b == "x";
|
||||
or_tf := a == "k" or b == "x";
|
||||
or_ft := a == "z" or b == "v";
|
||||
print("or: {} {} {}\n", or_ff, or_tf, or_ft);
|
||||
|
||||
// string == feeding both `and` and `or` in one expression
|
||||
mixed := a == "k" and b == "v" or a == "z";
|
||||
print("mixed: {}\n", mixed);
|
||||
|
||||
// string `!=` operands too
|
||||
ne := a != "z" and b != "z";
|
||||
print("ne: {}\n", ne);
|
||||
|
||||
// the larger shape: a match-expression value plus an enum-payload string
|
||||
// == combined under `and`/`or`.
|
||||
v : Json = .str("v");
|
||||
kind := if v == {
|
||||
case .str: 1;
|
||||
case .int_: 2;
|
||||
case .null_: 3;
|
||||
}
|
||||
ok := kind == 1 and v.str == "v";
|
||||
bad := kind == 2 or v.str == "x";
|
||||
print("payload: {} {}\n", ok, bad);
|
||||
}
|
||||
@@ -0,0 +1 @@
|
||||
0
|
||||
@@ -0,0 +1 @@
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
and: true false false
|
||||
or: false true true
|
||||
mixed: true
|
||||
ne: true
|
||||
payload: true false
|
||||
@@ -1,3 +1,21 @@
|
||||
# 0078 — string `==` as an `and`/`or` operand emits an invalid PHI
|
||||
|
||||
> **RESOLVED.** Root cause was in the LLVM emitter, not the `and`/`or`
|
||||
> lowering: `fixupPhiNodes` wired each short-circuit merge PHI's incoming
|
||||
> edge to `block_map[ir_block]` — the LLVM block the IR block *started* as.
|
||||
> But a single IR instruction can expand into its own sub-CFG during
|
||||
> emission (string `==`'s `str.memcmp`/`str.merge` blocks; a value `match`'s
|
||||
> arm blocks), leaving the builder in a later block. The terminator — and
|
||||
> therefore the real predecessor edge — lands in that later block, so the
|
||||
> recorded predecessor was stale (`%entry`/`%and.rhs.0` instead of
|
||||
> `%str.merge`). Fix: in `src/ir/emit_llvm.zig`, record the builder's
|
||||
> *actual* insertion block after emitting each IR block's instructions
|
||||
> (`term_block_map`, captured via `LLVMGetInsertBlock`) and use that as the
|
||||
> PHI predecessor in `fixupPhiNodes`. General — corrects the incoming block
|
||||
> for ANY operand that emitted intermediate basic blocks, not just string
|
||||
> `==`. Mirrors the issue-0066 "stale PHI incoming-block after an operand
|
||||
> emits new blocks" shape. Regression: `examples/0045-basic-string-eq-short-circuit.sx`.
|
||||
|
||||
# Symptom
|
||||
|
||||
A string equality (`a == "x"`) used as an operand of a short-circuit
|
||||
|
||||
@@ -124,6 +124,13 @@ pub const LLVMEmitter = struct {
|
||||
|
||||
// Maps (func_idx, block_idx) → LLVM BasicBlock
|
||||
block_map: std.AutoHashMap(u64, c.LLVMBasicBlockRef),
|
||||
// For each IR block, the LLVM block its terminator was actually emitted
|
||||
// into. Usually equals `block_map[block]`, but an instruction can expand
|
||||
// into its own sub-CFG (string `==`'s memcmp blocks, a value `match`'s
|
||||
// arm blocks) and leave the builder in a later block, so the terminator —
|
||||
// and therefore the PHI predecessor edge — lands there instead. Keyed the
|
||||
// same way as `block_map`.
|
||||
term_block_map: std.AutoHashMap(u64, c.LLVMBasicBlockRef),
|
||||
|
||||
// Cached LLVM types
|
||||
cached_i1: c.LLVMTypeRef,
|
||||
@@ -278,6 +285,7 @@ pub const LLVMEmitter = struct {
|
||||
.func_map = std.AutoHashMap(u32, c.LLVMValueRef).init(alloc),
|
||||
.global_map = std.AutoHashMap(u32, c.LLVMValueRef).init(alloc),
|
||||
.block_map = std.AutoHashMap(u64, c.LLVMBasicBlockRef).init(alloc),
|
||||
.term_block_map = std.AutoHashMap(u64, c.LLVMBasicBlockRef).init(alloc),
|
||||
.pending_phis = std.ArrayList(PendingPhi).empty,
|
||||
.cached_i1 = c.LLVMInt1TypeInContext(ctx),
|
||||
.cached_i8 = c.LLVMInt8TypeInContext(ctx),
|
||||
@@ -311,6 +319,7 @@ pub const LLVMEmitter = struct {
|
||||
self.jni_slots.deinit();
|
||||
self.global_map.deinit();
|
||||
self.block_map.deinit();
|
||||
self.term_block_map.deinit();
|
||||
self.di_files.deinit();
|
||||
var fsc_it = self.frame_str_cache.keyIterator();
|
||||
while (fsc_it.next()) |k| self.alloc.free(k.*);
|
||||
@@ -1270,6 +1279,7 @@ pub const LLVMEmitter = struct {
|
||||
|
||||
// Clear pending phis for this function
|
||||
self.pending_phis.clearRetainingCapacity();
|
||||
self.term_block_map.clearRetainingCapacity();
|
||||
|
||||
// Emit instructions for each block — use first_ref to sync ref numbering
|
||||
for (func.blocks.items, 0..) |block, bi| {
|
||||
@@ -1285,6 +1295,12 @@ pub const LLVMEmitter = struct {
|
||||
_ = inst_i;
|
||||
self.emitInst(&instruction, func_idx);
|
||||
}
|
||||
|
||||
// The terminator may have landed in a later LLVM block than `bb`
|
||||
// if an instruction in this IR block expanded into its own sub-CFG.
|
||||
// Record where the builder actually is so PHI predecessors point at
|
||||
// the block that holds the branch, not the block we started in.
|
||||
self.term_block_map.put(block_key, c.LLVMGetInsertBlock(self.builder)) catch unreachable;
|
||||
}
|
||||
|
||||
// Fixup PHI nodes: scan all blocks for branches that pass args
|
||||
@@ -1300,7 +1316,10 @@ pub const LLVMEmitter = struct {
|
||||
|
||||
for (func.blocks.items, 0..) |block, bi| {
|
||||
const src_key = makeBlockKey(func_idx, @intCast(bi));
|
||||
const src_bb = self.block_map.get(src_key) orelse continue;
|
||||
// Predecessor is the block the terminator was emitted into, which
|
||||
// differs from `block_map[bi]` when an instruction expanded the
|
||||
// block into a sub-CFG (string `==`, value `match`, …).
|
||||
const src_bb = self.term_block_map.get(src_key) orelse continue;
|
||||
|
||||
for (block.insts.items) |instruction| {
|
||||
switch (instruction.op) {
|
||||
|
||||
Reference in New Issue
Block a user