From 1e8cdd599cbb0037bb37b858b1fc6c026d434373 Mon Sep 17 00:00:00 2001 From: Wesley Peck Date: Mon, 6 Dec 2010 21:11:01 +0000 Subject: [PATCH] Fix a 16-bit immediate value detection bug in the MBlaze delay slot filler. Address more hazards in the MBlaze delay slot filler. patch contributed by Jack Whitham! git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@121037 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/MBlaze/MBlazeDelaySlotFiller.cpp | 134 +++++++++++++------- test/CodeGen/MBlaze/imm.ll | 2 +- 2 files changed, 87 insertions(+), 49 deletions(-) diff --git a/lib/Target/MBlaze/MBlazeDelaySlotFiller.cpp b/lib/Target/MBlaze/MBlazeDelaySlotFiller.cpp index 6a819c28bab..024d3d55fcc 100644 --- a/lib/Target/MBlaze/MBlazeDelaySlotFiller.cpp +++ b/lib/Target/MBlaze/MBlazeDelaySlotFiller.cpp @@ -29,6 +29,14 @@ using namespace llvm; STATISTIC(FilledSlots, "Number of delay slots filled"); +namespace llvm { +cl::opt DisableDelaySlotFiller( + "disable-mblaze-delay-filler", + cl::init(false), + cl::desc("Disable the MBlaze delay slot filter."), + cl::Hidden); +} + namespace { struct Filler : public MachineFunctionPass { @@ -61,15 +69,22 @@ static bool hasImmInstruction(MachineBasicBlock::iterator &candidate) { // 16-bits requires an implicit IMM instruction. unsigned numOper = candidate->getNumOperands(); for (unsigned op = 0; op < numOper; ++op) { - if (candidate->getOperand(op).isImm() && - (candidate->getOperand(op).getImm() & 0xFFFFFFFFFFFF0000LL) != 0) - return true; + MachineOperand &mop = candidate->getOperand(op); + + // The operand requires more than 16-bits to represent. + if (mop.isImm() && (mop.getImm() < -0x8000 || mop.getImm() > 0x7fff)) + return true; + + // We must assume that unknown immediate values require more than + // 16-bits to represent. + if (mop.isGlobal() || mop.isSymbol()) + return true; // FIXME: we could probably check to see if the FP value happens // to not need an IMM instruction. For now we just always - // assume that FP values always do. - if (candidate->getOperand(op).isFPImm()) - return true; + // assume that FP values do. + if (mop.isFPImm()) + return true; } return false; @@ -77,48 +92,67 @@ static bool hasImmInstruction(MachineBasicBlock::iterator &candidate) { static bool delayHasHazard(MachineBasicBlock::iterator &candidate, MachineBasicBlock::iterator &slot) { - // Loop over all of the operands in the branch instruction - // and make sure that none of them are defined by the - // candidate instruction. - unsigned numOper = slot->getNumOperands(); - for (unsigned op = 0; op < numOper; ++op) { - if (!slot->getOperand(op).isReg() || - !slot->getOperand(op).isUse() || - slot->getOperand(op).isImplicit()) - continue; - - unsigned cnumOper = candidate->getNumOperands(); - for (unsigned cop = 0; cop < cnumOper; ++cop) { - if (candidate->getOperand(cop).isReg() && - candidate->getOperand(cop).isDef() && - candidate->getOperand(cop).getReg() == - slot->getOperand(op).getReg()) - return true; - } + // Hazard check + MachineBasicBlock::iterator a = candidate; + MachineBasicBlock::iterator b = slot; + TargetInstrDesc desc = candidate->getDesc(); + + // MBB layout:- + // candidate := a0 = operation(a1, a2) + // ...middle bit... + // slot := b0 = operation(b1, b2) + + // Possible hazards:-/ + // 1. a1 or a2 was written during the middle bit + // 2. a0 was read or written during the middle bit + // 3. a0 is one or more of {b0, b1, b2} + // 4. b0 is one or more of {a1, a2} + // 5. a accesses memory, and the middle bit + // contains a store operation. + bool a_is_memory = desc.mayLoad() || desc.mayStore(); + + // Check hazards type 1, 2 and 5 by scanning the middle bit + MachineBasicBlock::iterator m = a; + for (++m; m != b; ++m) { + for (unsigned aop = 0, aend = a->getNumOperands(); aopgetOperand(aop).isReg(); + if (!aop_is_reg) continue; + + bool aop_is_def = a->getOperand(aop).isDef(); + unsigned aop_reg = a->getOperand(aop).getReg(); + + for (unsigned mop = 0, mend = m->getNumOperands(); mopgetOperand(mop).isReg(); + if (!mop_is_reg) continue; + + bool mop_is_def = m->getOperand(mop).isDef(); + unsigned mop_reg = m->getOperand(mop).getReg(); + + if (aop_is_def && (mop_reg == aop_reg)) + return true; // Hazard type 2, because aop = a0 + else if (mop_is_def && (mop_reg == aop_reg)) + return true; // Hazard type 1, because aop in {a1, a2} + } } - // There are no hazards between the two instructions - return false; -} + // Check hazard type 5 + if (a_is_memory && m->getDesc().mayStore()) + return true; + } + + // Check hazard type 3 & 4 + for (unsigned aop = 0, aend = a->getNumOperands(); aopgetOperand(aop).isReg()) { + unsigned aop_reg = a->getOperand(aop).getReg(); -static bool usedBeforeDelaySlot(MachineBasicBlock::iterator &candidate, - MachineBasicBlock::iterator &slot) { - MachineBasicBlock::iterator I = candidate; - for (++I; I != slot; ++I) { - unsigned numOper = I->getNumOperands(); - for (unsigned op = 0; op < numOper; ++op) { - if (I->getOperand(op).isReg() && - I->getOperand(op).isUse()) { - unsigned reg = I->getOperand(op).getReg(); - unsigned cops = candidate->getNumOperands(); - for (unsigned cop = 0; cop < cops; ++cop) { - if (candidate->getOperand(cop).isReg() && - candidate->getOperand(cop).isDef() && - candidate->getOperand(cop).getReg() == reg) - return true; - } - } + for (unsigned bop = 0, bend = b->getNumOperands(); bopgetOperand(bop).isReg() && (!b->getOperand(bop).isImplicit())) { + unsigned bop_reg = b->getOperand(bop).getReg(); + if (aop_reg == bop_reg) + return true; } + } + } } return false; @@ -142,11 +176,12 @@ findDelayInstr(MachineBasicBlock &MBB,MachineBasicBlock::iterator slot) { --I; TargetInstrDesc desc = I->getDesc(); - if (desc.hasDelaySlot() || desc.isBranch() || isDelayFiller(MBB,I)) + if (desc.hasDelaySlot() || desc.isBranch() || isDelayFiller(MBB,I) || + desc.isCall() || desc.isReturn() || desc.isBarrier() || + desc.hasUnmodeledSideEffects()) break; - if (desc.mayLoad() || desc.mayStore() || hasImmInstruction(I) || - delayHasHazard(I,slot) || usedBeforeDelaySlot(I,slot)) + if (hasImmInstruction(I) || delayHasHazard(I,slot)) continue; return I; @@ -162,9 +197,12 @@ bool Filler::runOnMachineBasicBlock(MachineBasicBlock &MBB) { bool Changed = false; for (MachineBasicBlock::iterator I = MBB.begin(); I != MBB.end(); ++I) if (I->getDesc().hasDelaySlot()) { - MachineBasicBlock::iterator D = findDelayInstr(MBB,I); + MachineBasicBlock::iterator D = MBB.end(); MachineBasicBlock::iterator J = I; + if (!DisableDelaySlotFiller) + D = findDelayInstr(MBB,I); + ++FilledSlots; Changed = true; diff --git a/test/CodeGen/MBlaze/imm.ll b/test/CodeGen/MBlaze/imm.ll index 7574687f8cd..6effd3e09a2 100644 --- a/test/CodeGen/MBlaze/imm.ll +++ b/test/CodeGen/MBlaze/imm.ll @@ -22,7 +22,7 @@ define i16 @retimm_i16() { ; FPU: retimm_i16: ; FPU: rtsd ; FPU-NEXT: add - ret i16 38212 + ret i16 31212 } define i32 @retimm_i32() { -- 2.34.1