Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
b0c3aed
Add ControlFlowType enum, ControlFlowMarker class, and RuntimeControl…
fglock Nov 6, 2025
e7b78e9
Modify EmitControlFlow to return marked RuntimeList for non-local con…
fglock Nov 6, 2025
48d5cd5
Add control flow check infrastructure (disabled) - WIP Phase 3
fglock Nov 6, 2025
10f631b
Phase 2 complete: Non-local control flow via tagged returns (99.8%)
fglock Nov 6, 2025
937c0b3
Fix plan: Skip Phase 3 (call-site checks), renumber to make straightf…
fglock Nov 6, 2025
86daaee
Phase 3 partial: Add TAILCALL trampoline at returnLabel (99.8% mainta…
fglock Nov 6, 2025
c03f76f
Phase 3: Tail call trampoline working, call-site checks deferred
fglock Nov 6, 2025
28f39a0
feat: Add feature flags and Phase 3 loop handler infrastructure
fglock Nov 6, 2025
0d1cccb
feat: Implement Phase 4 (Top-Level Safety)
fglock Nov 6, 2025
4a8a369
feat: Propagate isMainProgram flag to code generation
fglock Nov 6, 2025
1f5bc8b
feat: Add source location tracking to control flow markers
fglock Nov 6, 2025
5544bbc
feat: Pass source location to RuntimeControlFlowList
fglock Nov 6, 2025
065bc87
docs: Streamline design doc to action-oriented plan
fglock Nov 6, 2025
c4db263
feat: Dynamic slot allocation for control flow temp storage
fglock Nov 6, 2025
a183087
docs: Update plan with ASM frame computation findings
fglock Nov 6, 2025
e8c5a85
docs: Add critical decision document for control flow architecture
fglock Nov 6, 2025
e3bcbad
docs: Document existing SKIP workarounds to be removed
fglock Nov 6, 2025
b9ec4d0
fix: Remove top-level safety check - restored 99.8% pass rate
fglock Nov 6, 2025
0405780
docs: Update plan - Phases 5 & 6 complete (99.9% pass rate)
fglock Nov 6, 2025
c5c895d
test: Add comprehensive control flow unit tests
fglock Nov 6, 2025
fe226a3
Fix goto __SUB__ tail call by detecting it in handleGotoLabel
fglock Nov 6, 2025
326a856
Update MILESTONES.md: goto __SUB__ is now working
fglock Nov 6, 2025
bb0e5fe
Update design document: Phase 7 (Tail Call Trampoline) COMPLETE
fglock Nov 6, 2025
8c674dd
Add comprehensive comments explaining disabled features and next steps
fglock Nov 6, 2025
d5477dd
Update MILESTONES.md: Mark Phase 7 (Non-local control flow) as COMPLETE
fglock Nov 6, 2025
84084ac
Update FEATURE_MATRIX.md: Add tail call features, simplify control fl…
fglock Nov 6, 2025
055630b
docs
fglock Nov 6, 2025
cb28263
fix: Prevent RuntimeControlFlowList from being corrupted as data
fglock Nov 6, 2025
7cc0bf1
docs: Document RuntimeControlFlowList data corruption fix
fglock Nov 6, 2025
2d03783
docs: Comprehensive analysis of ASM frame computation blocker
fglock Nov 6, 2025
ee4cb82
feat: Implement runtime control flow registry for 'last SKIP' support
fglock Nov 6, 2025
b2e23ef
docs: Add comprehensive documentation for control flow registry solution
fglock Nov 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add comprehensive comments explaining disabled features and next steps
Added detailed comments to all feature flags explaining:
- What each disabled feature would do if enabled
- Why it's currently disabled (ASM frame computation issues)
- What investigation is needed to fix it
- Dependencies between features

Updated design document to focus on next steps:
- Option A: Profile first (recommended) - avoid premature optimization
- Option B: Fix call-site checks (3 approaches with effort estimates)
- Option C: Enable loop handlers (after B works)

The current implementation works correctly at 99.9% pass rate.
Call-site checks and loop handlers are optional performance optimizations.
  • Loading branch information
fglock committed Nov 6, 2025
commit 8c674dd831c842862749bd70caf858c924f8aa57
126 changes: 78 additions & 48 deletions dev/design/TAGGED_RETURN_CONTROL_FLOW.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,64 +46,94 @@ timeout 900 dev/tools/perl_test_runner.pl \

---

## NEXT STEPS - Implementation Order
## Current Status (as of Phase 7 completion)

### **CURRENT: Phase 3 - Fix Call-Site Checks (BLOCKED)**
**Working Features:** ✓
- All Perl control flow: `last`/`next`/`redo`/`goto LABEL`/`goto &NAME`/`goto __SUB__`
- Local control flow uses plain JVM GOTO (zero overhead)
- Non-local control flow uses tagged returns (propagates through return paths)
- Tail call optimization via trampoline at `returnLabel`
- Pass rate: **99.9%** (1778/1779 tests)

**STATUS:** ✗ BLOCKED by ASM frame computation issues
**Disabled Optimizations:**
- Call-site checks (blocked by ASM frame computation issues)
- Loop handlers (depends on call-site checks)

**COMPLETED:**
- ✓ Dynamic slot allocation (`controlFlowTempSlot`)
- ✓ Simplified pattern (store→check→branch)
- ✗ Still fails with `ArrayIndexOutOfBoundsException` in ASM Frame.merge()

**ROOT CAUSE:** ASM's COMPUTE_FRAMES cannot handle inline branching checks that manipulate the stack. The pattern `DUP → ASTORE → ALOAD → instanceof → branch → pop/cleanup → GOTO` creates control flow paths that ASM cannot compute frames for, even with simplified patterns.

**WHY THIS IS HARD:**
- Call sites are in expression context (value on stack)
- Need to check value without consuming it (requires DUP)
- Need to clean stack if marked (requires knowing stack depth)
- Branching + stack manipulation = ASM frame computation failure

**ALTERNATIVE APPROACHES TO INVESTIGATE:**

1. **Explicit Frame Hints** (High effort, error-prone)
- Manually call `mv.visitFrame()` at every branch point
- Requires tracking exact local variable types and stack types
- Fragile - breaks if bytecode changes

2. **VOID Context Checks** (Promising!)
- Only check in VOID context (after value is POPped)
- No DUP needed, no stack manipulation
- But: marked returns would be lost in VOID context
- Would need to store EVERY call result temporarily

3. **No Inline Checks - Return Path Only** (RECOMMENDED)
- Remove inline call-site checks entirely
- Only check at `returnLabel` (global return point)
- Loop handlers become "optional optimization" for nested loops
- Non-local control flow ALWAYS returns to caller, caller checks and re-dispatches
- **This matches how exceptions work!**

**RECOMMENDED PATH FORWARD:**
Skip call-site checks for now. Focus on validating that Phase 2 (creating marked returns) and Phase 5 (top-level safety in RuntimeCode.apply()) work correctly. Call-site checks can be added later as an optimization if needed.

**TEST:** Basic control flow already works without call-site checks:
```bash
./jperl -e 'for (1..3) { print "$_\n"; last; }' # ✓ Works!
```
**Performance:**
- Local jumps: zero overhead (plain JVM GOTO)
- Non-local jumps: propagate through return paths (acceptable overhead, rare in practice)

---

### **Phase 4 - Enable Loop Handlers (SKIPPED)**
## NEXT STEPS - Optional Optimizations

These are performance optimizations, not correctness fixes. The system works correctly at 99.9% pass rate.

### **Option A: Profile First (RECOMMENDED)**

**Goal:** Determine if optimization is worth the effort.

**Steps:**
1. Profile real Perl code (DynaLoader, Test::More, benchmarks)
2. Measure overhead of current implementation
3. **If < 1% overhead:** Stop - current implementation is good
4. **If > 5% overhead:** Proceed to Option B

**Why First:** Avoid premature optimization. Current implementation works at 99.9%.

---

### **Option B: Fix Call-Site Checks (If profiling shows need)**

**Previous Attempts:**
- ✗ Dynamic slot allocation - failed with `ArrayIndexOutOfBoundsException`
- ✗ Simplified pattern (store→check→branch) - still fails ASM frame merge

**Root Cause:** ASM cannot handle inline branching + stack manipulation pattern.

**Investigations to Try:**

**B1. Static Slot Pre-Allocation** (30 min, low risk)
```java
// At method entry (like tailCallCodeRefSlot):
controlFlowCheckSlot = symbolTable.allocateLocalVariable();
```
- Pre-allocate slot before any branches
- May fix "slot allocated after branch" issue

**B2. Store-Then-Check Pattern** (1 hour, medium risk)
```java
ASTORE tempSlot // Store directly (no DUP)
ALOAD tempSlot // Load for check
INSTANCEOF ... // Check
// ...
ALOAD tempSlot // Reload for normal path
```
- Extra ALOAD but simpler control flow

**B3. Manual Frame Hints** (4+ hours, high risk)
```java
mv.visitFrame(F_SAME, ...); // Explicit frame at merge
```
- Direct control over ASM
- Fragile, requires deep ASM knowledge
- Last resort only

**Recommended:** Try B1, then B2. Skip B3 unless critical.

---

**STATUS:** ✗ SKIPPED - depends on call-site checks which are blocked
### **Option C: Enable Loop Handlers (After B works)**

**WHY NEEDED:** Loop handlers would optimize nested loops by processing marked returns immediately instead of propagating to caller.
**Depends On:** Call-site checks working (Option B complete)

**WHY SKIPPED:** Loop handlers are only reachable via call-site checks. Without call-site checks, the handler code is unreachable (dead code) and ASM frame computation fails.
**Steps:**
1. Enable `ENABLE_CONTROL_FLOW_CHECKS = true`
2. Enable `ENABLE_LOOP_HANDLERS = true`
3. Test with `make`
4. Debug any VerifyErrors with `--disassemble`

**ALTERNATIVE:** Non-local control flow propagates through return path. `RuntimeCode.apply()` catches it at the top level. This works but is less efficient for deeply nested loops.
**Effort:** 2-4 hours

---

Expand Down
25 changes: 22 additions & 3 deletions src/main/java/org/perlonjava/codegen/EmitForeach.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,28 @@

public class EmitForeach {
// Feature flags for control flow implementation
// Set to true to enable control flow handlers for foreach loops (Phase 3)
// DISABLED: Requires call-site checks which are currently broken (ASM frame computation issues)
// TODO: Fix call-site checks first, then re-enable loop handlers
//
// WHAT THIS WOULD DO IF ENABLED:
// After a foreach loop body executes, if a subroutine call returned a RuntimeControlFlowList
// (marked with last/next/redo/goto), execution jumps to a controlFlowHandler that:
// 1. Checks the control flow type (LAST/NEXT/REDO/GOTO/TAILCALL)
// 2. Checks if the label matches this loop
// 3. If match: jumps to the appropriate loop label (lastLabel/nextLabel/redoLabel/gotoLabel)
// 4. If no match: propagates to parent loop's handler (for nested loops)
// 5. If no parent: returns to caller with the marked list
//
// WHY IT'S DISABLED:
// Loop handlers are only reachable via call-site checks (ENABLE_CONTROL_FLOW_CHECKS).
// Without call-site checks jumping to the handler, this code is unreachable (dead code).
// Dead code can confuse ASM's frame computation and cause verification errors.
//
// DEPENDENCY:
// This flag MUST remain false until ENABLE_CONTROL_FLOW_CHECKS in EmitSubroutine.java
// is fixed and enabled. See comments there for investigation steps.
//
// CURRENT WORKAROUND:
// Marked returns propagate through normal return paths all the way to the top level.
// Local control flow (within the same loop) still uses fast JVM GOTO instructions.
private static final boolean ENABLE_LOOP_HANDLERS = false;

// Set to true to enable debug output for loop control flow
Expand Down
11 changes: 10 additions & 1 deletion src/main/java/org/perlonjava/codegen/EmitStatement.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,16 @@
*/
public class EmitStatement {
// Feature flags for control flow implementation
// Set to true to enable control flow handlers for loops (Phase 3)
// Feature flag for loop control flow handlers (same as EmitForeach.java)
//
// WHAT THIS WOULD DO IF ENABLED:
// Enable control flow handlers for while/until/for loops (same as foreach).
//
// WHY IT'S DISABLED:
// Same reason as EmitForeach.java - depends on ENABLE_CONTROL_FLOW_CHECKS.
//
// DEPENDENCY:
// Must remain false until EmitSubroutine.ENABLE_CONTROL_FLOW_CHECKS is fixed.
private static final boolean ENABLE_LOOP_HANDLERS = false;

// Set to true to enable debug output for loop control flow
Expand Down
35 changes: 32 additions & 3 deletions src/main/java/org/perlonjava/codegen/EmitSubroutine.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,38 @@
*/
public class EmitSubroutine {
// Feature flags for control flow implementation
// Flag to enable/disable control flow checks at call sites (Phase 7 - required for loop handlers)
// DISABLED: ASM frame computation cannot handle complex branching with stack manipulation
// TODO: Need different architecture - maybe explicit frame hints or simpler pattern
//
// WHAT THIS WOULD DO IF ENABLED:
// After every subroutine call, check if the returned RuntimeList is a RuntimeControlFlowList
// (marked with last/next/redo/goto), and if so, immediately propagate it to returnLabel
// instead of continuing execution. This would enable loop handlers to catch control flow
// at the loop level instead of propagating all the way up the call stack.
//
// WHY IT'S DISABLED:
// The inline check pattern causes ArrayIndexOutOfBoundsException in ASM's frame computation:
// DUP // Duplicate result
// INSTANCEOF RuntimeControlFlowList // Check if marked
// IFEQ notMarked // Branch
// ASTORE tempSlot // Store (slot allocated dynamically)
// emitPopInstructions(0) // Clean stack
// ALOAD tempSlot // Restore
// GOTO returnLabel // Propagate
// notMarked: POP // Discard duplicate
//
// The complex branching with dynamic slot allocation breaks ASM's ability to merge frames
// at the branch target, especially when the tempSlot is allocated after the branch instruction.
//
// INVESTIGATION NEEDED:
// 1. Try allocating tempSlot statically at method entry (not dynamically per call)
// 2. Try simpler pattern without DUP (accept performance hit of extra ALOAD/ASTORE)
// 3. Try manual frame hints with visitFrame() at merge points
// 4. Consider moving check to VOID context only (after POP) - but this loses marked returns
// 5. Profile real-world code to see if this optimization actually matters
//
// CURRENT WORKAROUND:
// Without call-site checks, marked returns propagate through normal return paths.
// This works correctly but is less efficient for deeply nested loops crossing subroutines.
// Performance impact is minimal since most control flow is local (uses plain JVM GOTO).
private static final boolean ENABLE_CONTROL_FLOW_CHECKS = false;

// Set to true to enable debug output for control flow checks
Expand Down