From 24a9ec6cf63ebac2e5e59b6dbdb8f1ff1d73fd6d Mon Sep 17 00:00:00 2001 From: yurai007 Date: Thu, 29 Feb 2024 22:55:40 +0100 Subject: [PATCH] JIT: Deduplicate CodeGen::genJumpTable (#99110) Moving same code used across all architectures to common codegen should simplify things and reduce possibility of future regressions as noted in #98992. --- src/coreclr/jit/codegen.h | 1 + src/coreclr/jit/codegenarm.cpp | 25 +--------------- src/coreclr/jit/codegenarm64.cpp | 28 +----------------- src/coreclr/jit/codegencommon.cpp | 41 ++++++++++++++++++++++++++ src/coreclr/jit/codegenloongarch64.cpp | 28 +----------------- src/coreclr/jit/codegenriscv64.cpp | 28 +----------------- src/coreclr/jit/codegenxarch.cpp | 28 +----------------- 7 files changed, 47 insertions(+), 132 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 4d17e291d0c8a..7a2359c9fb5fc 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -1260,6 +1260,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX void genCodeForInitBlkLoop(GenTreeBlk* initBlkNode); void genCodeForInitBlkRepStos(GenTreeBlk* initBlkNode); void genCodeForInitBlkUnroll(GenTreeBlk* initBlkNode); + unsigned genEmitJumpTable(GenTree* treeNode, bool relativeAddr); void genJumpTable(GenTree* tree); void genTableBasedSwitch(GenTree* tree); #if defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) diff --git a/src/coreclr/jit/codegenarm.cpp b/src/coreclr/jit/codegenarm.cpp index eefee0e6912cf..8cf3ac32b3a32 100644 --- a/src/coreclr/jit/codegenarm.cpp +++ b/src/coreclr/jit/codegenarm.cpp @@ -647,30 +647,7 @@ void CodeGen::genTableBasedSwitch(GenTree* treeNode) // void CodeGen::genJumpTable(GenTree* treeNode) { - noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH)); - assert(treeNode->OperGet() == GT_JMPTABLE); - - unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount; - FlowEdge** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab; - unsigned jmpTabBase; - - jmpTabBase = GetEmitter()->emitBBTableDataGenBeg(jumpCount, false); - - JITDUMP("\n J_M%03u_DS%02u LABEL DWORD\n", compiler->compMethodID, jmpTabBase); - - for (unsigned i = 0; i < jumpCount; i++) - { - BasicBlock* target = (*jumpTable)->getDestinationBlock(); - jumpTable++; - noway_assert(target->HasFlag(BBF_HAS_LABEL)); - - JITDUMP(" DD L_M%03u_" FMT_BB "\n", compiler->compMethodID, target->bbNum); - - GetEmitter()->emitDataGenData(i, target); - } - - GetEmitter()->emitDataGenEnd(); - + unsigned jmpTabBase = genEmitJumpTable(treeNode, false); genMov32RelocatableDataLabel(jmpTabBase, treeNode->GetRegNum()); genProduceReg(treeNode); diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 4587bace1697a..498f227e48d5b 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -3750,33 +3750,7 @@ void CodeGen::genTableBasedSwitch(GenTree* treeNode) // emits the table and an instruction to get the address of the first element void CodeGen::genJumpTable(GenTree* treeNode) { - noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH)); - assert(treeNode->OperGet() == GT_JMPTABLE); - - unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount; - FlowEdge** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab; - unsigned jmpTabOffs; - unsigned jmpTabBase; - - jmpTabBase = GetEmitter()->emitBBTableDataGenBeg(jumpCount, true); - - jmpTabOffs = 0; - - JITDUMP("\n J_M%03u_DS%02u LABEL DWORD\n", compiler->compMethodID, jmpTabBase); - - for (unsigned i = 0; i < jumpCount; i++) - { - BasicBlock* target = (*jumpTable)->getDestinationBlock(); - jumpTable++; - noway_assert(target->HasFlag(BBF_HAS_LABEL)); - - JITDUMP(" DD L_M%03u_" FMT_BB "\n", compiler->compMethodID, target->bbNum); - - GetEmitter()->emitDataGenData(i, target); - }; - - GetEmitter()->emitDataGenEnd(); - + unsigned jmpTabBase = genEmitJumpTable(treeNode, true); // Access to inline data is 'abstracted' by a special type of static member // (produced by eeFindJitDataOffs) which the emitter recognizes as being a reference // to constant data, not a real static field. diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 9d7992ea4efe2..bf2bfaa28aad4 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -6452,6 +6452,47 @@ void CodeGen::genFnProlog() #pragma warning(pop) #endif +//---------------------------------------------------------------------------------- +// genEmitJumpTable: emit jump table and return its base offset +// +// Arguments: +// treeNode - the GT_JMPTABLE node +// relativeAddr - if true, references are treated as 4-byte relative addresses, +// otherwise they are absolute pointers +// +// Return Value: +// base offset to jump table +// +// Assumption: +// The current basic block in process ends with a switch statement +// +unsigned CodeGen::genEmitJumpTable(GenTree* treeNode, bool relativeAddr) +{ + noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH)); + assert(treeNode->OperGet() == GT_JMPTABLE); + + emitter* emit = GetEmitter(); + const unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount; + FlowEdge** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab; + const unsigned jmpTabBase = emit->emitBBTableDataGenBeg(jumpCount, relativeAddr); + + JITDUMP("\n J_M%03u_DS%02u LABEL DWORD\n", compiler->compMethodID, jmpTabBase); + + for (unsigned i = 0; i < jumpCount; i++) + { + BasicBlock* target = (*jumpTable)->getDestinationBlock(); + jumpTable++; + noway_assert(target->HasFlag(BBF_HAS_LABEL)); + + JITDUMP(" DD L_M%03u_" FMT_BB "\n", compiler->compMethodID, target->bbNum); + + emit->emitDataGenData(i, target); + }; + + emit->emitDataGenEnd(); + return jmpTabBase; +} + //------------------------------------------------------------------------ // getCallTarget - Get the node that evaluates to the call target // diff --git a/src/coreclr/jit/codegenloongarch64.cpp b/src/coreclr/jit/codegenloongarch64.cpp index e894d98763331..01e82be425a3c 100644 --- a/src/coreclr/jit/codegenloongarch64.cpp +++ b/src/coreclr/jit/codegenloongarch64.cpp @@ -2927,33 +2927,7 @@ void CodeGen::genTableBasedSwitch(GenTree* treeNode) // emits the table and an instruction to get the address of the first element void CodeGen::genJumpTable(GenTree* treeNode) { - noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH)); - assert(treeNode->OperGet() == GT_JMPTABLE); - - unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount; - FlowEdge** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab; - unsigned jmpTabOffs; - unsigned jmpTabBase; - - jmpTabBase = GetEmitter()->emitBBTableDataGenBeg(jumpCount, true); - - jmpTabOffs = 0; - - JITDUMP("\n J_M%03u_DS%02u LABEL DWORD\n", compiler->compMethodID, jmpTabBase); - - for (unsigned i = 0; i < jumpCount; i++) - { - BasicBlock* target = (*jumpTable)->getDestinationBlock(); - jumpTable++; - noway_assert(target->HasFlag(BBF_HAS_LABEL)); - - JITDUMP(" DD L_M%03u_" FMT_BB "\n", compiler->compMethodID, target->bbNum); - - GetEmitter()->emitDataGenData(i, target); - }; - - GetEmitter()->emitDataGenEnd(); - + unsigned jmpTabBase = genEmitJumpTable(treeNode, true); // Access to inline data is 'abstracted' by a special type of static member // (produced by eeFindJitDataOffs) which the emitter recognizes as being a reference // to constant data, not a real static field. diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp index 90429b89584fd..c66511fa87b8a 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -2850,33 +2850,7 @@ void CodeGen::genTableBasedSwitch(GenTree* treeNode) // emits the table and an instruction to get the address of the first element void CodeGen::genJumpTable(GenTree* treeNode) { - noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH)); - assert(treeNode->OperGet() == GT_JMPTABLE); - - unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount; - FlowEdge** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab; - unsigned jmpTabOffs; - unsigned jmpTabBase; - - jmpTabBase = GetEmitter()->emitBBTableDataGenBeg(jumpCount, true); - - jmpTabOffs = 0; - - JITDUMP("\n J_M%03u_DS%02u LABEL DWORD\n", compiler->compMethodID, jmpTabBase); - - for (unsigned i = 0; i < jumpCount; i++) - { - BasicBlock* target = (*jumpTable)->getDestinationBlock(); - jumpTable++; - noway_assert(target->HasFlag(BBF_HAS_LABEL)); - - JITDUMP(" DD L_M%03u_" FMT_BB "\n", compiler->compMethodID, target->bbNum); - - GetEmitter()->emitDataGenData(i, target); - }; - - GetEmitter()->emitDataGenEnd(); - + unsigned jmpTabBase = genEmitJumpTable(treeNode, true); // Access to inline data is 'abstracted' by a special type of static member // (produced by eeFindJitDataOffs) which the emitter recognizes as being a reference // to constant data, not a real static field. diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 584cb3aab19bb..223199f35c327 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -4303,33 +4303,7 @@ void CodeGen::genTableBasedSwitch(GenTree* treeNode) // emits the table and an instruction to get the address of the first element void CodeGen::genJumpTable(GenTree* treeNode) { - noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH)); - assert(treeNode->OperGet() == GT_JMPTABLE); - - unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount; - FlowEdge** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab; - unsigned jmpTabOffs; - unsigned jmpTabBase; - - jmpTabBase = GetEmitter()->emitBBTableDataGenBeg(jumpCount, true); - - jmpTabOffs = 0; - - JITDUMP("\n J_M%03u_DS%02u LABEL DWORD\n", compiler->compMethodID, jmpTabBase); - - for (unsigned i = 0; i < jumpCount; i++) - { - BasicBlock* target = (*jumpTable)->getDestinationBlock(); - jumpTable++; - noway_assert(target->HasFlag(BBF_HAS_LABEL)); - - JITDUMP(" DD L_M%03u_" FMT_BB "\n", compiler->compMethodID, target->bbNum); - - GetEmitter()->emitDataGenData(i, target); - }; - - GetEmitter()->emitDataGenEnd(); - + unsigned jmpTabBase = genEmitJumpTable(treeNode, true); // Access to inline data is 'abstracted' by a special type of static member // (produced by eeFindJitDataOffs) which the emitter recognizes as being a reference // to constant data, not a real static field.