From 18eab554dea086710534e84dc0f3539c5028cf77 Mon Sep 17 00:00:00 2001 From: Vladimir Radosavljevic Date: Wed, 4 Dec 2024 14:49:21 +0100 Subject: [PATCH] [EVM] Introduce pseudo jumps, call and ret instructions This patch adds pseudo jumps, call and ret instructions to fix machine verifier after stackification and to reduce complexity added with bundles. Signed-off-by: Vladimir Radosavljevic --- llvm/lib/Target/EVM/EVMAsmPrinter.cpp | 63 +++++++-------- llvm/lib/Target/EVM/EVMAssembly.cpp | 81 ++++++------------- llvm/lib/Target/EVM/EVMAssembly.h | 4 +- llvm/lib/Target/EVM/EVMInstrInfo.td | 14 +++- .../Target/EVM/EVMOptimizedCodeTransform.cpp | 2 +- llvm/lib/Target/EVM/EVMStackify.cpp | 47 +++-------- llvm/test/CodeGen/EVM/fallthrough.mir | 45 ----------- 7 files changed, 82 insertions(+), 174 deletions(-) delete mode 100644 llvm/test/CodeGen/EVM/fallthrough.mir diff --git a/llvm/lib/Target/EVM/EVMAsmPrinter.cpp b/llvm/lib/Target/EVM/EVMAsmPrinter.cpp index 999ac042d2bd..ea39d5e3679a 100644 --- a/llvm/lib/Target/EVM/EVMAsmPrinter.cpp +++ b/llvm/lib/Target/EVM/EVMAsmPrinter.cpp @@ -46,39 +46,15 @@ class EVMAsmPrinter : public AsmPrinter { StringRef getPassName() const override { return "EVM Assembly "; } - void SetupMachineFunction(MachineFunction &MF) override; - void emitInstruction(const MachineInstr *MI) override; void emitFunctionEntryLabel() override; - /// Return true if the basic block has exactly one predecessor and the control - /// transfer mechanism between the predecessor and this block is a - /// fall-through. - bool isBlockOnlyReachableByFallthrough( - const MachineBasicBlock *MBB) const override; - private: void emitLinkerSymbol(const MachineInstr *MI); }; } // end of anonymous namespace -void EVMAsmPrinter::SetupMachineFunction(MachineFunction &MF) { - // Unbundle bundles. - for (MachineBasicBlock &MBB : MF) { - MachineBasicBlock::instr_iterator I = MBB.instr_begin(), - E = MBB.instr_end(); - for (; I != E; ++I) { - if (I->isBundledWithPred()) { - assert(I->isConditionalBranch() || I->isUnconditionalBranch()); - I->unbundleFromPred(); - } - } - } - - AsmPrinter::SetupMachineFunction(MF); -} - void EVMAsmPrinter::emitFunctionEntryLabel() { AsmPrinter::emitFunctionEntryLabel(); @@ -105,8 +81,37 @@ void EVMAsmPrinter::emitInstruction(const MachineInstr *MI) { EVMMCInstLower MCInstLowering(OutContext, *this, VRegMapping, MF->getRegInfo()); - unsigned Opc = MI->getOpcode(); - if (Opc == EVM::DATASIZE_S || Opc == EVM::DATAOFFSET_S) { + switch (MI->getOpcode()) { + default: + break; + case EVM::PseudoCALL: + case EVM::PseudoRET: { + // TODO: #746: Use PseudoInstExpansion and do this expansion in tblgen. + MCInst Jump; + Jump.setOpcode(EVM::JUMP_S); + EmitToStreamer(*OutStreamer, Jump); + return; + } + case EVM::PseudoJUMP: + case EVM::PseudoJUMPI: { + MCInst Push; + Push.setOpcode(EVM::PUSH4_S); + + // TODO: #745: Refactor EVMMCInstLower::Lower so we could use lowerOperand + // instead of creating a MCOperand directly. + MCOperand MCOp = MCOperand::createExpr(MCSymbolRefExpr::create( + MI->getOperand(0).getMBB()->getSymbol(), OutContext)); + Push.addOperand(MCOp); + EmitToStreamer(*OutStreamer, Push); + + MCInst Jump; + Jump.setOpcode(MI->getOpcode() == EVM::PseudoJUMP ? EVM::JUMP_S + : EVM::JUMPI_S); + EmitToStreamer(*OutStreamer, Jump); + return; + } + case EVM::DATASIZE_S: + case EVM::DATAOFFSET_S: emitLinkerSymbol(MI); return; } @@ -116,12 +121,6 @@ void EVMAsmPrinter::emitInstruction(const MachineInstr *MI) { EmitToStreamer(*OutStreamer, TmpInst); } -bool EVMAsmPrinter::isBlockOnlyReachableByFallthrough( - const MachineBasicBlock *MBB) const { - // For simplicity, always emit BB labels. - return false; -} - void EVMAsmPrinter::emitLinkerSymbol(const MachineInstr *MI) { MCSymbol *LinkerSymbol = MI->getOperand(0).getMCSymbol(); StringRef LinkerSymbolName = LinkerSymbol->getName(); diff --git a/llvm/lib/Target/EVM/EVMAssembly.cpp b/llvm/lib/Target/EVM/EVMAssembly.cpp index 21a7020e284c..775af0097186 100644 --- a/llvm/lib/Target/EVM/EVMAssembly.cpp +++ b/llvm/lib/Target/EVM/EVMAssembly.cpp @@ -136,15 +136,6 @@ void EVMAssembly::appendLabelReference(MCSymbol *Label) { CurMIIt = std::next(CurMIIt); } -void EVMAssembly::appendMBBReference(MachineBasicBlock *MBB) { - CurMIIt = BuildMI(*CurMBB, CurMIIt, DebugLoc(), TII->get(EVM::PUSH_LABEL)) - .addMBB(MBB); - StackHeight += 1; - AssemblyInstrs.insert(&*CurMIIt); - LLVM_DEBUG(dumpInst(&*CurMIIt)); - CurMIIt = std::next(CurMIIt); -} - MCSymbol *EVMAssembly::createFuncRetSymbol() { return MF->getContext().createTempSymbol("FUNC_RET", true); } @@ -165,9 +156,9 @@ void EVMAssembly::appendFuncCall(const MachineInstr *MI, LLVM_DEBUG(dumpInst(&*CurMIIt)); CurMIIt = std::next(CurMIIt); - // Create jump to the callee. Note, we don't add the 'target' operand to JUMP. - // This should be fine, unless we run MachineVerifier after this step - CurMIIt = BuildMI(*CurMBB, CurMIIt, MI->getDebugLoc(), TII->get(EVM::JUMP)); + // Create jump to the callee. + CurMIIt = + BuildMI(*CurMBB, CurMIIt, MI->getDebugLoc(), TII->get(EVM::PseudoCALL)); if (RetSym) CurMIIt->setPostInstrSymbol(*MF, RetSym); AssemblyInstrs.insert(&*CurMIIt); @@ -175,9 +166,8 @@ void EVMAssembly::appendFuncCall(const MachineInstr *MI, CurMIIt = std::next(CurMIIt); } -void EVMAssembly::appendJump(int StackAdj) { - CurMIIt = BuildMI(*CurMBB, CurMIIt, DebugLoc(), TII->get(EVM::JUMP)); - StackHeight += StackAdj; +void EVMAssembly::appendRet() { + CurMIIt = BuildMI(*CurMBB, CurMIIt, DebugLoc(), TII->get(EVM::PseudoRET)); AssemblyInstrs.insert(&*CurMIIt); LLVM_DEBUG(dumpInst(&*CurMIIt)); CurMIIt = std::next(CurMIIt); @@ -186,22 +176,25 @@ void EVMAssembly::appendJump(int StackAdj) { void EVMAssembly::appendUncondJump(MachineInstr *MI, MachineBasicBlock *Target) { assert(MI->getOpcode() == EVM::JUMP); - appendMBBReference(Target); - [[maybe_unused]] auto It = AssemblyInstrs.insert(MI); - assert(It.second && StackHeight > 0); - StackHeight -= 1; - LLVM_DEBUG(dumpInst(MI)); - CurMIIt = std::next(MIIter(MI)); + CurMIIt = + BuildMI(*CurMBB, CurMIIt, MI->getDebugLoc(), TII->get(EVM::PseudoJUMP)) + .addMBB(Target); + assert(StackHeight >= 0); + AssemblyInstrs.insert(&*CurMIIt); + LLVM_DEBUG(dumpInst(&*CurMIIt)); + CurMIIt = std::next(CurMIIt); } void EVMAssembly::appendCondJump(MachineInstr *MI, MachineBasicBlock *Target) { assert(MI->getOpcode() == EVM::JUMPI); - appendMBBReference(Target); - [[maybe_unused]] auto It = AssemblyInstrs.insert(MI); - assert(It.second && StackHeight > 1); - StackHeight -= 2; - LLVM_DEBUG(dumpInst(MI)); - CurMIIt = std::next(MIIter(MI)); + CurMIIt = + BuildMI(*CurMBB, CurMIIt, MI->getDebugLoc(), TII->get(EVM::PseudoJUMPI)) + .addMBB(Target); + StackHeight -= 1; + assert(StackHeight >= 0); + AssemblyInstrs.insert(&*CurMIIt); + LLVM_DEBUG(dumpInst(&*CurMIIt)); + CurMIIt = std::next(CurMIIt); } // Remove all registers operands of the \p MI and repaces the opcode with @@ -211,7 +204,9 @@ void EVMAssembly::stackifyInstruction(MachineInstr *MI) { return; unsigned RegOpcode = MI->getOpcode(); - if (RegOpcode == EVM::PUSH_LABEL) + if (RegOpcode == EVM::PUSH_LABEL || RegOpcode == EVM::PseudoJUMP || + RegOpcode == EVM::PseudoJUMPI || RegOpcode == EVM::PseudoCALL || + RegOpcode == EVM::PseudoRET) return; // Remove register operands. @@ -233,8 +228,7 @@ void EVMAssembly::finalize() { SmallVector ToRemove; for (MachineBasicBlock &MBB : *MF) { for (MachineInstr &MI : MBB) { - if (!AssemblyInstrs.count(&MI) && MI.getOpcode() != EVM::JUMP && - MI.getOpcode() != EVM::JUMPI) + if (!AssemblyInstrs.count(&MI)) ToRemove.emplace_back(&MI); } } @@ -253,31 +247,4 @@ void EVMAssembly::finalize() { // In a stackified code register liveness has no meaning. MachineRegisterInfo &MRI = MF->getRegInfo(); MRI.invalidateLiveness(); - - // In EVM architecture jump target is set up using one of PUSH* instructions - // that come right before the jump instruction. - // For example: - - // PUSH_LABEL %bb.10 - // JUMPI_S - // PUSH_LABEL %bb.9 - // JUMP_S - // - // The problem here is that such MIR is not valid. There should not be - // non-terminator (PUSH) instructions between terminator (JUMP) ones. - // To overcome this issue, we bundle adjacent instructions - // together and unbundle them in the AsmPrinter. - for (MachineBasicBlock &MBB : *MF) { - MachineBasicBlock::instr_iterator I = MBB.instr_begin(), - E = MBB.instr_end(); - // Skip the first instruction, as it's not interested anyway. - ++I; - for (; I != E; ++I) { - if (I->isBranch()) { - auto P = std::prev(I); - if (P->getOpcode() == EVM::PUSH_LABEL) - I->bundleWithPred(); - } - } - } } diff --git a/llvm/lib/Target/EVM/EVMAssembly.h b/llvm/lib/Target/EVM/EVMAssembly.h index 8f40acfebe9e..58c0e2ed36de 100644 --- a/llvm/lib/Target/EVM/EVMAssembly.h +++ b/llvm/lib/Target/EVM/EVMAssembly.h @@ -71,7 +71,7 @@ class EVMAssembly { void appendFuncCall(const MachineInstr *MI, const llvm::GlobalValue *Func, int stackAdj, MCSymbol *RetSym = nullptr); - void appendJump(int stackAdj); + void appendRet(); void appendCondJump(MachineInstr *MI, MachineBasicBlock *Target); @@ -79,8 +79,6 @@ class EVMAssembly { void appendLabelReference(MCSymbol *Label); - void appendMBBReference(MachineBasicBlock *MBB); - MCSymbol *createFuncRetSymbol(); // Erases unused codegen-only instructions and removes register operands diff --git a/llvm/lib/Target/EVM/EVMInstrInfo.td b/llvm/lib/Target/EVM/EVMInstrInfo.td index c79609c167e7..e2776d53e697 100644 --- a/llvm/lib/Target/EVM/EVMInstrInfo.td +++ b/llvm/lib/Target/EVM/EVMInstrInfo.td @@ -248,6 +248,8 @@ let variadicOpsAreDefs = 1 in def FCALL : NRI<(outs), (ins jmptarget:$callee, variable_ops), [], "FCALL\t$callee">; +let isPseudo = 1 in +def PseudoCALL : EVMPseudo<(outs), (ins), []>; } // Uses = [SP], isCall = 1 @@ -405,18 +407,26 @@ let isBranch = 1, isTerminator = 1 in { defm JUMPI : I<(outs), (ins jmptarget:$dst, GPR:$cond), [(brcond GPR:$cond, bb:$dst)], "JUMPI", " $dst, $cond", 0x57, 10>; +let isPseudo = 1 in +def PseudoJUMPI : EVMPseudo<(outs), (ins jmptarget:$dst), []>; -let isBarrier = 1 in +let isBarrier = 1 in { defm JUMP : I<(outs), (ins jmptarget:$dst), [(br bb:$dst)], "JUMP", " $dst", 0x56, 8>; +let isPseudo = 1 in +def PseudoJUMP : EVMPseudo<(outs), (ins jmptarget:$dst), []>; +} // isBarrier = 1 } // isBranch = 1, isTerminator = 1 // This isn't really a control flow instruction, but it should be used to mark // destination of jump instructions. defm JUMPDEST : I<(outs), (ins), [], "JUMPDEST", "", 0x5B, 1>; -let isBarrier = 1, isTerminator = 1, isReturn = 1 in +let isBarrier = 1, isTerminator = 1, isReturn = 1 in { def RET : NRI<(outs), (ins variable_ops), [(EVMret)], "RET">; +let isPseudo = 1 in +def PseudoRET : EVMPseudo<(outs), (ins), []>; +} //===----------------------------------------------------------------------===// diff --git a/llvm/lib/Target/EVM/EVMOptimizedCodeTransform.cpp b/llvm/lib/Target/EVM/EVMOptimizedCodeTransform.cpp index 9b79e094b971..046067ee640f 100644 --- a/llvm/lib/Target/EVM/EVMOptimizedCodeTransform.cpp +++ b/llvm/lib/Target/EVM/EVMOptimizedCodeTransform.cpp @@ -442,7 +442,7 @@ void EVMOptimizedCodeTransform::operator()(const CFG::BasicBlock &Block) { // Create the function return layout and jump. createStackLayout(ExitStack); - Assembly.appendJump(0); + Assembly.appendRet(); }, [&](CFG::BasicBlock::Unreachable const &) { assert(Block.Operations.empty()); diff --git a/llvm/lib/Target/EVM/EVMStackify.cpp b/llvm/lib/Target/EVM/EVMStackify.cpp index 722078251f8f..145ff2d7d211 100644 --- a/llvm/lib/Target/EVM/EVMStackify.cpp +++ b/llvm/lib/Target/EVM/EVMStackify.cpp @@ -720,16 +720,20 @@ void StackModel::handleArgument(MachineInstr *MI) { void StackModel::handleLStackAtJump(MachineBasicBlock *MBB, MachineInstr *MI, const Register &Reg) { + assert(MI->getOpcode() == EVM::JUMP || MI->getOpcode() == EVM::JUMPI); + // If the condition register is in the L-stack, we need to move it to // the bottom of the L-stack. After that we should clean clean the L-stack. // In case of an unconditional jump, the Reg value should be // EVM::NoRegister. clearPhysStackAtInst(StackType::L, MI, Reg); - // Insert "PUSH_LABEL %bb" instruction that should be be replaced with - // the actual PUSH* one in the MC layer to contain actual jump target - // offset. - BuildMI(*MI->getParent(), MI, DebugLoc(), TII->get(EVM::PUSH_LABEL)) + // Insert pseudo jump instruciton that will be replaced with PUSH and JUMP + // instructions in AsmPrinter. + ToErase.push_back(MI); + unsigned PseudoJumpOpc = + MI->getOpcode() == EVM::JUMP ? EVM::PseudoJUMP : EVM::PseudoJUMPI; + BuildMI(*MI->getParent(), MI, DebugLoc(), TII->get(PseudoJumpOpc)) .addMBB(MBB); // Add JUMPDEST at the beginning of the target MBB. @@ -752,7 +756,7 @@ void StackModel::handleJump(MachineInstr *MI) { void StackModel::handleReturn(MachineInstr *MI) { ToErase.push_back(MI); BuildMI(*MI->getParent(), std::next(MIIter(MI)), DebugLoc(), - TII->get(EVM::JUMP)); + TII->get(EVM::PseudoRET)); // Collect the use registers of the RET instruction. SmallVector ReturnRegs; @@ -872,7 +876,7 @@ void StackModel::handleCall(MachineInstr *MI) { MCSymbol *RetSym = MF->getContext().createTempSymbol("FUNC_RET", true); // Create jump to the callee. - It = BuildMI(MBB, It, MI->getDebugLoc(), TII->get(EVM::JUMP)); + It = BuildMI(MBB, It, MI->getDebugLoc(), TII->get(EVM::PseudoCALL)); It->setPostInstrSymbol(*MF, RetSym); // Create push of the return address. @@ -995,7 +999,9 @@ void StackModel::stackifyInstruction(MachineInstr *MI) { return; unsigned RegOpcode = MI->getOpcode(); - if (RegOpcode == EVM::PUSH_LABEL) + if (RegOpcode == EVM::PUSH_LABEL || RegOpcode == EVM::PseudoJUMP || + RegOpcode == EVM::PseudoJUMPI || RegOpcode == EVM::PseudoCALL || + RegOpcode == EVM::PseudoRET) return; // Remove register operands. @@ -1048,33 +1054,6 @@ void StackModel::postProcess() { // In a stackified code register liveness has no meaning. MachineRegisterInfo &MRI = MF->getRegInfo(); MRI.invalidateLiveness(); - - // In EVM architecture jump target is set up using one of PUSH* instructions - // that come right before the jump instruction. - // For example: - - // PUSH_LABEL %bb.10 - // JUMPI_S - // PUSH_LABEL %bb.9 - // JUMP_S - // - // The problem here is that such MIR is not valid. There should not be - // non-terminator (PUSH) instructions between terminator (JUMP) ones. - // To overcome this issue, we bundle adjacent instructions - // together and unbundle them in the AsmPrinter. - for (MachineBasicBlock &MBB : *MF) { - MachineBasicBlock::instr_iterator I = MBB.instr_begin(), - E = MBB.instr_end(); - // Skip the first instruction, as it's not interested anyway. - ++I; - for (; I != E; ++I) { - if (I->isBranch()) { - auto P = std::prev(I); - if (P->getOpcode() == EVM::PUSH_LABEL) - I->bundleWithPred(); - } - } - } } void StackModel::dumpState() const { diff --git a/llvm/test/CodeGen/EVM/fallthrough.mir b/llvm/test/CodeGen/EVM/fallthrough.mir deleted file mode 100644 index 1e578fb13513..000000000000 --- a/llvm/test/CodeGen/EVM/fallthrough.mir +++ /dev/null @@ -1,45 +0,0 @@ -# RUN: llc -x mir --start-after=evm-stackify < %s | FileCheck %s - - ---- | - - target datalayout = "E-p:256:256-i256:256:256-S256-a:256:256" - target triple = "evm" - define void @test_fallthrough() { ret void } - -... ---- -# CHECK: PUSH4 @.BB0_1 -# CHECK-NEXT: JUMPI -# CHECK-NEXT: PUSH4 @.BB0_2 -# CHECK-NEXT: JUMP -# CHECK-NEXT: .BB0_1: - -name: test_fallthrough -tracksRegLiveness: false -machineFunctionInfo: - isStackified: true -body: | - bb.0: - JUMPDEST_S - PUSH_LABEL %bb.10, implicit-def $arguments { - JUMPI_S - } - PUSH_LABEL %bb.13, implicit-def $arguments { - JUMP_S - } - - bb.10: - liveins: $value_stack - JUMPDEST_S - PUSH0_S - PUSH0_S - REVERT_S - - bb.13: - liveins: $value_stack - JUMPDEST_S - PUSH0_S - PUSH0_S - REVERT_S -...