From 02fb59d8a19442cc10eae4a1e12ec4eb3804551e Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 14 May 2026 19:17:15 -0700 Subject: [PATCH 1/2] [Wasm/Ryujit] Fix if stack depth; fix pep call ordering Account for additional control flow stack depth added during codegen when using `if/end`. Fix argument eval ordering for PEP calls so the PEP is pushed as the last arg, and then pushed and indirected to get the function index. --- src/coreclr/jit/codegen.h | 7 +++-- src/coreclr/jit/codegenwasm.cpp | 45 ++++++++++++++++++++++++++------- src/coreclr/jit/lowerwasm.cpp | 12 +++++++-- 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index bd15513f199d71..3772b378de62ab 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -214,8 +214,9 @@ class CodeGen final : public CodeGenInterface void genCodeForBlock(BasicBlock* block); #if defined(TARGET_WASM) - ArrayStack* wasmControlFlowStack = nullptr; - unsigned wasmCursor = 0; + ArrayStack* wasmControlFlowStack = nullptr; + unsigned wasmCursor = 0; + unsigned wasmExtraControlFlowDepth = 0; unsigned findTargetDepth(BasicBlock* target); void WasmProduceReg(GenTree* node); regNumber GetMultiUseOperandReg(GenTree* operand); @@ -223,6 +224,8 @@ class CodeGen final : public CodeGenInterface unsigned GetStackPointerRegIndex() const; unsigned GetFramePointerRegIndex() const; void ensureCurrentFuncIsUnwindable(); + void genEmitIf(); + void genEmitEndIf(); #endif void genEmitStartBlock(BasicBlock* block); diff --git a/src/coreclr/jit/codegenwasm.cpp b/src/coreclr/jit/codegenwasm.cpp index eeba91f5bb40be..5216c2a15f43bb 100644 --- a/src/coreclr/jit/codegenwasm.cpp +++ b/src/coreclr/jit/codegenwasm.cpp @@ -1459,7 +1459,7 @@ void CodeGen::genCodeForBinaryOverflow(GenTreeOp* treeNode) // TODO-WASM-CQ: consider branchless alternative here (and for sub) GetEmitter()->emitIns_I(is64BitOp ? INS_i64_const : INS_i32_const, emitActualTypeSize(treeNode), 0); GetEmitter()->emitIns(is64BitOp ? INS_i64_ge_s : INS_i32_ge_s); - GetEmitter()->emitIns(INS_if); + genEmitIf(); { // Operands have the same sign. If the sum has a different sign, then the add overflowed. GetEmitter()->emitIns_I(INS_local_get, emitActualTypeSize(treeNode), WasmRegToIndex(resultReg)); @@ -1469,7 +1469,7 @@ void CodeGen::genCodeForBinaryOverflow(GenTreeOp* treeNode) GetEmitter()->emitIns(is64BitOp ? INS_i64_lt_s : INS_i32_lt_s); genJumpToThrowHlpBlk(SCK_OVERFLOW); } - GetEmitter()->emitIns(INS_end); + genEmitEndIf(); GetEmitter()->emitIns_I(INS_local_get, emitActualTypeSize(treeNode), WasmRegToIndex(resultReg)); break; } @@ -1490,7 +1490,7 @@ void CodeGen::genCodeForBinaryOverflow(GenTreeOp* treeNode) GetEmitter()->emitIns(is64BitOp ? INS_i64_xor : INS_i32_xor); GetEmitter()->emitIns_I(is64BitOp ? INS_i64_const : INS_i32_const, emitActualTypeSize(treeNode), 0); GetEmitter()->emitIns(is64BitOp ? INS_i64_lt_s : INS_i32_lt_s); - GetEmitter()->emitIns(INS_if); + genEmitIf(); { // Operands have different signs. If the difference has a different sign than op1, then the subtraction // overflowed. @@ -1501,7 +1501,7 @@ void CodeGen::genCodeForBinaryOverflow(GenTreeOp* treeNode) GetEmitter()->emitIns(is64BitOp ? INS_i64_lt_s : INS_i32_lt_s); genJumpToThrowHlpBlk(SCK_OVERFLOW); } - GetEmitter()->emitIns(INS_end); + genEmitEndIf(); GetEmitter()->emitIns_I(INS_local_get, emitActualTypeSize(treeNode), WasmRegToIndex(resultReg)); break; } @@ -1915,11 +1915,11 @@ void CodeGen::genJumpToThrowHlpBlk(SpecialCodeKind codeKind) } else { - GetEmitter()->emitIns_BlockTy(INS_if); + genEmitIf(); // Throw helpers are managed so we need to push the stack pointer before genEmitHelperCall. GetEmitter()->emitIns_I(INS_local_get, EA_PTRSIZE, GetStackPointerRegIndex()); genEmitHelperCall(m_compiler->acdHelper(codeKind), 0, EA_UNKNOWN); - GetEmitter()->emitIns(INS_end); + genEmitEndIf(); } } @@ -2996,7 +2996,7 @@ void CodeGen::genLclHeap(GenTree* tree) // Check for zero-sized requests GetEmitter()->emitIns(INS_I_eqz); - GetEmitter()->emitIns_BlockTy(INS_if, is64Bit ? WasmValueType::I64 : WasmValueType::I32); + genEmitIf(); { // If size is zero, leave a zero on the stack GetEmitter()->emitIns_I(INS_I_const, EA_PTRSIZE, 0); @@ -3053,7 +3053,7 @@ void CodeGen::genLclHeap(GenTree* tree) } // end if - GetEmitter()->emitIns(INS_end); + genEmitEndIf(); } WasmProduceReg(tree); @@ -3372,6 +3372,33 @@ void CodeGen::genLoadLocalIntoReg(regNumber targetReg, unsigned lclNum) GetEmitter()->emitIns_I(INS_local_set, emitTypeSize(type), WasmRegToIndex(targetReg)); } +//------------------------------------------------------------------------ +// genEmitIf: Emit an 'if' instruction +// +// Notes: +// This adds a new label to the control flow stack that is not +// modelled by the wasmControlFlowStack. Since we won't explicitly branch to the end +// of the `if` we just need to understand that the stack is now deeper. +// +void CodeGen::genEmitIf() +{ + wasmExtraControlFlowDepth++; + GetEmitter()->emitIns(INS_if); +} + +//------------------------------------------------------------------------ +// genEmitEndIf: Emit an 'end' instruction closing an 'if' block +// +// Notes: +// Removes the added stack depth from genEmitIf. +// +void CodeGen::genEmitEndIf() +{ + assert(wasmExtraControlFlowDepth > 0); + wasmExtraControlFlowDepth--; + GetEmitter()->emitIns(INS_end); +} + //------------------------------------------------------------------------ // inst_JMP: Emit a jump instruction. // @@ -3382,7 +3409,7 @@ void CodeGen::genLoadLocalIntoReg(regNumber targetReg, unsigned lclNum) void CodeGen::inst_JMP(emitJumpKind jmp, BasicBlock* tgtBlock) { instruction instr = emitter::emitJumpKindToIns(jmp); - unsigned const depth = findTargetDepth(tgtBlock); + unsigned const depth = findTargetDepth(tgtBlock) + wasmExtraControlFlowDepth; GetEmitter()->emitIns_J(instr, EA_4BYTE, depth, tgtBlock); } diff --git a/src/coreclr/jit/lowerwasm.cpp b/src/coreclr/jit/lowerwasm.cpp index 2d1b1895790753..3c341fff135bb7 100644 --- a/src/coreclr/jit/lowerwasm.cpp +++ b/src/coreclr/jit/lowerwasm.cpp @@ -96,10 +96,18 @@ void Lowering::LowerPEPCall(GenTreeCall* call) DISPTREE(call); JITDUMP("Rewrite PEP call's control expression to indirect through the new local variable\n"); + // Rewrite the call's control expression to have an additional load from the PEP local + // This must happen just before the call. + // GenTree* controlExpr = call->gtControlExpr; - GenTree* target = Ind(controlExpr); - BlockRange().InsertAfter(controlExpr, target); + assert(controlExpr->OperIs(GT_LCL_VAR)); + + BlockRange().Remove(controlExpr); + BlockRange().InsertBefore(call, controlExpr); + GenTree* target = Ind(controlExpr); + BlockRange().InsertBefore(call, target); + call->gtControlExpr = target; JITDUMP("Finished lowering PEP call\n"); From a04303142729696c041a8ad3d803e024fd8f2509 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 15 May 2026 07:29:12 -0700 Subject: [PATCH 2/2] add back ability to emit a typed if --- src/coreclr/jit/codegen.h | 2 +- src/coreclr/jit/codegenwasm.cpp | 20 +++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 3772b378de62ab..7bd09cb66d0d14 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -224,7 +224,7 @@ class CodeGen final : public CodeGenInterface unsigned GetStackPointerRegIndex() const; unsigned GetFramePointerRegIndex() const; void ensureCurrentFuncIsUnwindable(); - void genEmitIf(); + void genEmitIf(WasmValueType blockType = WasmValueType::Invalid); void genEmitEndIf(); #endif diff --git a/src/coreclr/jit/codegenwasm.cpp b/src/coreclr/jit/codegenwasm.cpp index 5216c2a15f43bb..7559d7b339bf7e 100644 --- a/src/coreclr/jit/codegenwasm.cpp +++ b/src/coreclr/jit/codegenwasm.cpp @@ -2975,7 +2975,6 @@ void CodeGen::genLclHeap(GenTree* tree) } else { - bool const is64Bit = (TARGET_POINTER_SIZE == 8); genConsumeReg(size); // Extend size to pointer size, if necessary @@ -2996,7 +2995,7 @@ void CodeGen::genLclHeap(GenTree* tree) // Check for zero-sized requests GetEmitter()->emitIns(INS_I_eqz); - genEmitIf(); + genEmitIf(WasmValueType::I); { // If size is zero, leave a zero on the stack GetEmitter()->emitIns_I(INS_I_const, EA_PTRSIZE, 0); @@ -3375,15 +3374,26 @@ void CodeGen::genLoadLocalIntoReg(regNumber targetReg, unsigned lclNum) //------------------------------------------------------------------------ // genEmitIf: Emit an 'if' instruction // +// Arguments: +// blockType - simple type for the block, or invalid if none +// // Notes: -// This adds a new label to the control flow stack that is not +// This emits an `if` instruction and adds a new label to the control flow stack that is not // modelled by the wasmControlFlowStack. Since we won't explicitly branch to the end // of the `if` we just need to understand that the stack is now deeper. // -void CodeGen::genEmitIf() +void CodeGen::genEmitIf(WasmValueType blockType) { wasmExtraControlFlowDepth++; - GetEmitter()->emitIns(INS_if); + + if (blockType != WasmValueType::Invalid) + { + GetEmitter()->emitIns_BlockTy(INS_if, blockType); + } + else + { + GetEmitter()->emitIns(INS_if); + } } //------------------------------------------------------------------------