diff --git a/changelog.d/8126-loop-poll-guard-cfg-assert.md b/changelog.d/8126-loop-poll-guard-cfg-assert.md new file mode 100644 index 0000000000..ed3e876af5 --- /dev/null +++ b/changelog.d/8126-loop-poll-guard-cfg-assert.md @@ -0,0 +1,15 @@ +`loop_safepoint_purity`'s guard assertion no longer depends on block print +order. `a_surviving_poll_is_guarded_by_the_arming_word` failed on main while +the lowering was correct: every `load volatile @PERRY_GC_POLL_ARMED` is +followed by `icmp` and `br i1` into a `gcpoll.N` block holding the call. + +The test searched the whole remainder of the IR after each load and required +the first `br i1` to precede the first poll call, which assumed each poll block +is printed next to its guard. It is not — the guards sit inline in `for.cond` / +`for.body` / `for.update` while the `gcpoll.N` blocks are emitted together +after the loop — so the leading segments contained no call, `find` returned +`None`, and a correctly-guarded poll was rejected. + +The check is now bounded at the load's own basic block: that block must end in +a conditional branch and must not contain the call. The negative controls are +unchanged and still pass, so the assertion keeps its teeth. diff --git a/crates/perry-codegen/tests/loop_safepoint_purity.rs b/crates/perry-codegen/tests/loop_safepoint_purity.rs index 1ca85e0752..372fb8bf56 100644 --- a/crates/perry-codegen/tests/loop_safepoint_purity.rs +++ b/crates/perry-codegen/tests/loop_safepoint_purity.rs @@ -457,11 +457,21 @@ fn a_surviving_poll_is_guarded_by_the_arming_word() { .split("load volatile i32, ptr @PERRY_GC_POLL_ARMED") .skip(1) .all(|after| { - // The `icmp` + `br` must come before the call: the call is on the - // taken arm, not in the same straight line as the load. - let call = after.find(POLL); - let branch = after.find("br i1 "); - matches!((call, branch), (Some(c), Some(b)) if b < c) + // The property is about the CFG, not about text order: the load's + // own block must end in a conditional branch, and the call must + // not be in that block — it belongs to a successor. + // + // Asking instead for "the first `br i1` appears before the first + // POLL anywhere after the load" silently depended on the poll + // blocks being emitted next to their guards. They are not: the + // guards sit inline in `for.cond` / `for.body` / `for.update`, + // while every `gcpoll.N` block is emitted together after the loop. + // Under that layout the first two segments contain no call at all, + // so the old check read `None` and failed a correctly-guarded + // poll. Bounding the search at the block edge asserts the real + // thing and does not care where the successor is printed. + let block_tail = after.split("\n\n").next().unwrap_or(after); + block_tail.contains("br i1 ") && !block_tail.contains(POLL) }); assert!( guarded,