From bb5a7442e362776621112dc9453e546a55878e79 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Thu, 29 Sep 2011 20:21:17 +0000 Subject: [PATCH] Clean up uses of switch instructions so they are not dependent on the operand ordering. Patch by Stepan Dyatkovskiy. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@140803 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Instructions.h | 7 +++++++ .../SelectionDAG/SelectionDAGBuilder.cpp | 4 ++-- lib/ExecutionEngine/Interpreter/Execution.cpp | 17 ++++++++++------- lib/Target/CBackend/CBackend.cpp | 15 +++++++++++---- lib/Target/CppBackend/CPPBackend.cpp | 17 ++++++++++------- .../InstCombine/InstructionCombining.cpp | 16 +++++++++++----- lib/Transforms/Utils/LowerSwitch.cpp | 4 ++-- lib/VMCore/AsmWriter.cpp | 14 ++++++++------ 8 files changed, 61 insertions(+), 33 deletions(-) diff --git a/include/llvm/Instructions.h b/include/llvm/Instructions.h index 2b681abbbdf..2f879a18617 100644 --- a/include/llvm/Instructions.h +++ b/include/llvm/Instructions.h @@ -2509,6 +2509,13 @@ public: return reinterpret_cast(getOperand(idx*2)); } + // setSuccessorValue - Updates the value associated with the specified + // successor. + void setSuccessorValue(unsigned idx, ConstantInt* SuccessorValue) { + assert(idx < getNumSuccessors() && "Successor # out of range!"); + setOperand(idx*2, reinterpret_cast(SuccessorValue)); + } + // Methods for support type inquiry through isa, cast, and dyn_cast: static inline bool classof(const SwitchInst *) { return true; } static inline bool classof(const Instruction *I) { diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 2f0ba856d16..59373d06937 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -2441,7 +2441,7 @@ void SelectionDAGBuilder::visitSwitch(const SwitchInst &SI) { // If there is only the default destination, branch to it if it is not the // next basic block. Otherwise, just fall through. - if (SI.getNumOperands() == 2) { + if (SI.getNumCases() == 1) { // Update machine-CFG edges. // If this is not a fall-through branch, emit the branch. @@ -2466,7 +2466,7 @@ void SelectionDAGBuilder::visitSwitch(const SwitchInst &SI) { // Get the Value to be switched on and default basic blocks, which will be // inserted into CaseBlock records, representing basic blocks in the binary // search tree. - const Value *SV = SI.getOperand(0); + const Value *SV = SI.getCondition(); // Push the initial CaseRec onto the worklist CaseRecVector WorkList; diff --git a/lib/ExecutionEngine/Interpreter/Execution.cpp b/lib/ExecutionEngine/Interpreter/Execution.cpp index 28fbf2b1593..27917da07a2 100644 --- a/lib/ExecutionEngine/Interpreter/Execution.cpp +++ b/lib/ExecutionEngine/Interpreter/Execution.cpp @@ -662,18 +662,21 @@ void Interpreter::visitBranchInst(BranchInst &I) { void Interpreter::visitSwitchInst(SwitchInst &I) { ExecutionContext &SF = ECStack.back(); - GenericValue CondVal = getOperandValue(I.getOperand(0), SF); - Type *ElTy = I.getOperand(0)->getType(); + Value* Cond = I.getCondition(); + Type *ElTy = Cond->getType(); + GenericValue CondVal = getOperandValue(Cond, SF); // Check to see if any of the cases match... BasicBlock *Dest = 0; - for (unsigned i = 2, e = I.getNumOperands(); i != e; i += 2) - if (executeICMP_EQ(CondVal, getOperandValue(I.getOperand(i), SF), ElTy) - .IntVal != 0) { - Dest = cast(I.getOperand(i+1)); + unsigned NumCases = I.getNumCases(); + // Skip the first item since that's the default case. + for (unsigned i = 1; i < NumCases; ++i) { + GenericValue CaseVal = getOperandValue(I.getCaseValue(i), SF); + if (executeICMP_EQ(CondVal, CaseVal, ElTy).IntVal != 0) { + Dest = cast(I.getSuccessor(i)); break; } - + } if (!Dest) Dest = I.getDefaultDest(); // No cases matched: use default SwitchToNewBasicBlock(Dest, SF); } diff --git a/lib/Target/CBackend/CBackend.cpp b/lib/Target/CBackend/CBackend.cpp index e3524e4c38d..020b80102eb 100644 --- a/lib/Target/CBackend/CBackend.cpp +++ b/lib/Target/CBackend/CBackend.cpp @@ -2383,22 +2383,29 @@ void CWriter::visitReturnInst(ReturnInst &I) { void CWriter::visitSwitchInst(SwitchInst &SI) { + Value* Cond = SI.getCondition(); + Out << " switch ("; - writeOperand(SI.getOperand(0)); + writeOperand(Cond); Out << ") {\n default:\n"; printPHICopiesForSuccessor (SI.getParent(), SI.getDefaultDest(), 2); printBranchToBlock(SI.getParent(), SI.getDefaultDest(), 2); Out << ";\n"; - for (unsigned i = 2, e = SI.getNumOperands(); i != e; i += 2) { + + unsigned NumCases = SI.getNumCases(); + // Skip the first item since that's the default case. + for (unsigned i = 1; i < NumCases; ++i) { + ConstantInt* CaseVal = SI.getCaseValue(i); + BasicBlock* Succ = SI.getSuccessor(i); Out << " case "; - writeOperand(SI.getOperand(i)); + writeOperand(CaseVal); Out << ":\n"; - BasicBlock *Succ = cast(SI.getOperand(i+1)); printPHICopiesForSuccessor (SI.getParent(), Succ, 2); printBranchToBlock(SI.getParent(), Succ, 2); if (Function::iterator(Succ) == llvm::next(Function::iterator(SI.getParent()))) Out << " break;\n"; } + Out << " }\n"; } diff --git a/lib/Target/CppBackend/CPPBackend.cpp b/lib/Target/CppBackend/CPPBackend.cpp index 8d6eda12046..16401a8f98b 100644 --- a/lib/Target/CppBackend/CPPBackend.cpp +++ b/lib/Target/CppBackend/CPPBackend.cpp @@ -154,7 +154,7 @@ namespace { void printFunctionHead(const Function *F); void printFunctionBody(const Function *F); void printInstruction(const Instruction *I, const std::string& bbname); - std::string getOpName(Value*); + std::string getOpName(const Value*); void printModuleBody(); }; @@ -979,7 +979,7 @@ void CppWriter::printVariableBody(const GlobalVariable *GV) { } } -std::string CppWriter::getOpName(Value* V) { +std::string CppWriter::getOpName(const Value* V) { if (!isa(V) || DefinedValues.find(V) != DefinedValues.end()) return getCppName(V); @@ -1044,14 +1044,17 @@ void CppWriter::printInstruction(const Instruction *I, case Instruction::Switch: { const SwitchInst *SI = cast(I); Out << "SwitchInst* " << iName << " = SwitchInst::Create(" - << opNames[0] << ", " - << opNames[1] << ", " + << getOpName(SI->getCondition()) << ", " + << getOpName(SI->getDefaultDest()) << ", " << SI->getNumCases() << ", " << bbname << ");"; nl(Out); - for (unsigned i = 2; i != SI->getNumOperands(); i += 2) { + unsigned NumCases = SI->getNumCases(); + for (unsigned i = 1; i < NumCases; ++i) { + const ConstantInt* CaseVal = SI->getCaseValue(i); + const BasicBlock* BB = SI->getSuccessor(i); Out << iName << "->addCase(" - << opNames[i] << ", " - << opNames[i+1] << ");"; + << getOpName(CaseVal) << ", " + << getOpName(BB) << ");"; nl(Out); } break; diff --git a/lib/Transforms/InstCombine/InstructionCombining.cpp b/lib/Transforms/InstCombine/InstructionCombining.cpp index f64990fe0b4..af2a5d2c1e9 100644 --- a/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -1237,11 +1237,17 @@ Instruction *InstCombiner::visitSwitchInst(SwitchInst &SI) { if (I->getOpcode() == Instruction::Add) if (ConstantInt *AddRHS = dyn_cast(I->getOperand(1))) { // change 'switch (X+4) case 1:' into 'switch (X) case -3' - for (unsigned i = 2, e = SI.getNumOperands(); i != e; i += 2) - SI.setOperand(i, - ConstantExpr::getSub(cast(SI.getOperand(i)), - AddRHS)); - SI.setOperand(0, I->getOperand(0)); + unsigned NumCases = SI.getNumCases(); + // Skip the first item since that's the default case. + for (unsigned i = 1; i < NumCases; ++i) { + ConstantInt* CaseVal = SI.getCaseValue(i); + Constant* NewCaseVal = ConstantExpr::getSub(cast(CaseVal), + AddRHS); + assert(isa(NewCaseVal) && + "Result of expression should be constant"); + SI.setSuccessorValue(i, cast(NewCaseVal)); + } + SI.setCondition(I->getOperand(0)); Worklist.Add(I); return &SI; } diff --git a/lib/Transforms/Utils/LowerSwitch.cpp b/lib/Transforms/Utils/LowerSwitch.cpp index ed733d393a1..686178ca01c 100644 --- a/lib/Transforms/Utils/LowerSwitch.cpp +++ b/lib/Transforms/Utils/LowerSwitch.cpp @@ -277,11 +277,11 @@ void LowerSwitch::processSwitchInst(SwitchInst *SI) { BasicBlock *CurBlock = SI->getParent(); BasicBlock *OrigBlock = CurBlock; Function *F = CurBlock->getParent(); - Value *Val = SI->getOperand(0); // The value we are switching on... + Value *Val = SI->getCondition(); // The value we are switching on... BasicBlock* Default = SI->getDefaultDest(); // If there is only the default destination, don't bother with the code below. - if (SI->getNumOperands() == 2) { + if (SI->getNumCases() == 1) { BranchInst::Create(SI->getDefaultDest(), CurBlock); CurBlock->getInstList().erase(SI); return; diff --git a/lib/VMCore/AsmWriter.cpp b/lib/VMCore/AsmWriter.cpp index 1fc94ba7ca2..64edc734c31 100644 --- a/lib/VMCore/AsmWriter.cpp +++ b/lib/VMCore/AsmWriter.cpp @@ -1702,18 +1702,20 @@ void AssemblyWriter::printInstruction(const Instruction &I) { writeOperand(BI.getSuccessor(1), true); } else if (isa(I)) { + SwitchInst& SI(cast(I)); // Special case switch instruction to get formatting nice and correct. Out << ' '; - writeOperand(Operand , true); + writeOperand(SI.getCondition(), true); Out << ", "; - writeOperand(I.getOperand(1), true); + writeOperand(SI.getDefaultDest(), true); Out << " ["; - - for (unsigned op = 2, Eop = I.getNumOperands(); op < Eop; op += 2) { + // Skip the first item since that's the default case. + unsigned NumCases = SI.getNumCases(); + for (unsigned i = 1; i < NumCases; ++i) { Out << "\n "; - writeOperand(I.getOperand(op ), true); + writeOperand(SI.getCaseValue(i), true); Out << ", "; - writeOperand(I.getOperand(op+1), true); + writeOperand(SI.getSuccessor(i), true); } Out << "\n ]"; } else if (isa(I)) {