[llvm-commits] [llvm] r44687 - in /llvm/trunk: include/llvm/CodeGen/Passes.h lib/CodeGen/LLVMTargetMachine.cpp lib/CodeGen/MachineLICM.cpp lib/Target/PowerPC/PPCInstrInfo.td
Evan Cheng
evan.cheng at apple.com
Fri Dec 7 16:42:59 PST 2007
On Dec 7, 2007, at 1:42 PM, Bill Wendling wrote:
>
>
> _foo:
> li r2, 0
> LBB1_1: ; bb
> li r5, 0
> stw r5, 0(r3)
> addi r2, r2, 1
> addi r3, r3, 4
> cmplw cr0, r2, r4
> bne cr0, LBB1_1 ; bb
> LBB1_2: ; return
> blr
>
> to:
>
> _foo:
> li r2, 0
> li r5, 0
> LBB1_1: ; bb
> stw r5, 0(r3)
> addi r2, r2, 1
> addi r3, r3, 4
> cmplw cr0, r2, r4
> bne cr0, LBB1_1 ; bb
> LBB1_2: ; return
> blr
>
> ZOMG!! :-)
Nicely done!
>
> +
> +#define DEBUG_TYPE "machine-licm"
> +#include "llvm/ADT/SmallVector.h"
> +#include "llvm/ADT/Statistic.h"
> +#include "llvm/CodeGen/MachineBasicBlock.h"
> +#include "llvm/CodeGen/MachineDominators.h"
> +#include "llvm/CodeGen/MachineInstr.h"
> +#include "llvm/CodeGen/MachineLoopInfo.h"
> +#include "llvm/CodeGen/Passes.h"
> +#include "llvm/Target/TargetInstrInfo.h"
> +#include "llvm/Support/CFG.h"
> +#include "llvm/Support/CommandLine.h"
> +#include "llvm/Support/Compiler.h"
> +#include "llvm/Support/Debug.h"
> +#include "llvm/Target/MRegisterInfo.h"
> +#include "llvm/Target/TargetMachine.h"
> +#include <map>
> +
> +using namespace llvm;
> +
> +namespace {
> + // Hidden options to help debugging
> + cl::opt<bool>
> + PerformLICM("machine-licm",
> + cl::init(false), cl::Hidden);
> +}
> +
> +namespace {
> + class VISIBILITY_HIDDEN MachineLICM : public MachineFunctionPass {
> + // Various analyses that we use...
> + MachineLoopInfo *LI; // Current MachineLoopInfo
> + MachineDominatorTree *DT; // Machine dominator tree for the
> current Loop
> +
> + const TargetInstrInfo *TII;
> +
> + // State that is updated as we process loops
> + bool Changed; // True if a loop is changed.
> + MachineLoop *CurLoop; // The current loop we are working
> on.
> +
> + // Map the def of a virtual register to the machine instruction.
> + std::map<unsigned, const MachineInstr*> VRegDefs;
Consider using IndexedMap.
>
> + public:
> + static char ID; // Pass identification, replacement for typeid
> + MachineLICM() : MachineFunctionPass((intptr_t)&ID) {}
> +
> + virtual bool runOnMachineFunction(MachineFunction &MF);
> +
> + /// FIXME: Loop preheaders?
> + ///
> + virtual void getAnalysisUsage(AnalysisUsage &AU) const {
> + AU.setPreservesCFG();
> + AU.addRequired<MachineLoopInfo>();
> + AU.addRequired<MachineDominatorTree>();
> + }
> + private:
> + /// GatherAllLoops - Get all loops in depth first order.
> + ///
> + void GatherAllLoops(MachineLoop *L,
> SmallVectorImpl<MachineLoop*> &Loops) {
> + const std::vector<MachineLoop*> &SubLoops = L->getSubLoops();
> +
> + for (MachineLoop::iterator
> + I = SubLoops.begin(), E = SubLoops.end(); I != E; ++I)
> + GatherAllLoops(*I, Loops);
> +
> + Loops.push_back(L);
> + }
> +
> + /// MapVirtualRegisterDefs - Create a map of which machine
> instruction
> + /// defines a virtual register.
> + ///
> + void MapVirtualRegisterDefs(const MachineFunction &MF);
> +
> + /// isInSubLoop - A little predicate that returns true if the
> specified
> + /// basic block is in a subloop of the current one, not the
> current one
> + /// itself.
> + ///
> + bool isInSubLoop(MachineBasicBlock *BB) {
> + assert(CurLoop->contains(BB) && "Only valid if BB is IN the
> loop");
> +
> + for (MachineLoop::iterator
> + I = CurLoop->begin(), E = CurLoop->end(); I != E; ++I)
> + if ((*I)->contains(BB))
> + return true; // A subloop actually contains this block!
> +
> + return false;
> + }
> +
> + /// CanHoistInst - Checks that this instructions is one that
> can be hoisted
> + /// out of the loop. I.e., it has no side effects, isn't a
> control flow
> + /// instr, etc.
> + ///
> + bool CanHoistInst(MachineInstr &I) const {
> + const TargetInstrDescriptor *TID = I.getInstrDescriptor();
> + MachineOpCode Opcode = TID->Opcode;
> +
> + return TII->isTriviallyReMaterializable(&I) &&
> + // FIXME: Below necessary?
> + !(TII->isReturn(Opcode) ||
> + TII->isTerminatorInstr(Opcode) ||
> + TII->isBranch(Opcode) ||
> + TII->isIndirectBranch(Opcode) ||
> + TII->isBarrier(Opcode) ||
> + TII->isCall(Opcode) ||
> + TII->isLoad(Opcode) || // TODO: Do loads and stores.
> + TII->isStore(Opcode));
> + }
Since you are touching this... When you have a chance, please rename
it to something like hasNoSideEffect().
>
> +
> + /// isLoopInvariantInst - Returns true if the instruction is loop
> + /// invariant. I.e., all virtual register operands are defined
> outside of
> + /// the loop, physical registers aren't accessed (explicitly or
> implicitly),
> + /// and the instruction is hoistable.
> + ///
> + bool isLoopInvariantInst(MachineInstr &I);
> +
> + /// FindPredecessors - Get all of the predecessors of the loop
> that are not
> + /// back-edges.
> + ///
> + void FindPredecessors(std::vector<MachineBasicBlock*> &Preds){
> + const MachineBasicBlock *Header = CurLoop->getHeader();
> +
> + for (MachineBasicBlock::const_pred_iterator
> + I = Header->pred_begin(), E = Header->pred_end(); I !=
> E; ++I)
> + if (!CurLoop->contains(*I))
> + Preds.push_back(*I);
> + }
> +
> + /// MoveInstToBlock - Moves the machine instruction to the
> bottom of the
> + /// predecessor basic block (but before the terminator
> instructions).
> + ///
> + void MoveInstToBlock(MachineBasicBlock *MBB, MachineInstr *MI) {
> + MachineBasicBlock::iterator Iter = MBB->getFirstTerminator();
> + MBB->insert(Iter, MI);
> + }
Poorly named. :-) MoveInstToEndOfBlock?
>
> +
> + /// HoistRegion - Walk the specified region of the CFG (defined
> by all
> + /// blocks dominated by the specified block, and that are in
> the current
> + /// loop) in depth first order w.r.t the DominatorTree. This
> allows us to
> + /// visit definitions before uses, allowing us to hoist a loop
> body in one
> + /// pass without iteration.
> + ///
> + void HoistRegion(MachineDomTreeNode *N);
>
> +
> + /// Hoist - When an instruction is found to only use loop
> invariant operands
> + /// that is safe to hoist, this instruction is called to do the
> dirty work.
> + ///
> + bool Hoist(MachineInstr &MI);
> + };
> +
> + char MachineLICM::ID = 0;
> + RegisterPass<MachineLICM> X("machine-licm",
> + "Machine Loop Invariant Code Motion");
> +} // end anonymous namespace
> +
> +FunctionPass *llvm::createMachineLICMPass() { return new
> MachineLICM(); }
> +
> +/// Hoist expressions out of the specified loop. Note, alias info
> for inner loop
> +/// is not preserved so it is not a good idea to run LICM multiple
> times on one
> +/// loop.
> +///
> +bool MachineLICM::runOnMachineFunction(MachineFunction &MF) {
> + if (!PerformLICM) return false; // For debugging.
> +
> + Changed = false;
> + TII = MF.getTarget().getInstrInfo();
> +
> + // Get our Loop information...
> + LI = &getAnalysis<MachineLoopInfo>();
> + DT = &getAnalysis<MachineDominatorTree>();
> +
> + for (MachineLoopInfo::iterator
> + I = LI->begin(), E = LI->end(); I != E; ++I) {
> + MachineLoop *L = *I;
> + CurLoop = L;
> +
> + // Visit all of the instructions of the loop. We want to visit
> the subloops
> + // first, though, so that we can hoist their invariants first
> into their
> + // containing loop before we process that loop.
> + SmallVector<MachineLoop*, 16> Loops;
> + GatherAllLoops(L, Loops);
Seems to me this can be smarter. When you are gathering the loops, put
loops of greater depth in the front of the queue then HoistRegion
won't have to check if the BB is in the current loop level?
>
> +
> + for (SmallVector<MachineLoop*, 8>::iterator
> + II = Loops.begin(), IE = Loops.end(); II != IE; ++II) {
> + L = *II;
> +
> + // Traverse the body of the loop in depth first order on the
> dominator
> + // tree so that we are guaranteed to see definitions before
> we see uses.
> + HoistRegion(DT->getNode(L->getHeader()));
> + }
> + }
> +
> + return Changed;
> +}
> +
> +/// MapVirtualRegisterDefs - Create a map of which machine
> instruction defines a
> +/// virtual register.
> +///
> +void MachineLICM::MapVirtualRegisterDefs(const MachineFunction &MF) {
> + for (MachineFunction::const_iterator
> + I = MF.begin(), E = MF.end(); I != E; ++I) {
> + const MachineBasicBlock &MBB = *I;
> +
> + for (MachineBasicBlock::const_iterator
> + II = MBB.begin(), IE = MBB.end(); II != IE; ++II) {
> + const MachineInstr &MI = *II;
> +
> + if (MI.getNumOperands() > 0) {
> + const MachineOperand &MO = MI.getOperand(0);
This is not right. You are assuming only one def and it must be the
first operand. Neither is necessarily true.
>
> +
> + if (MO.isRegister() && MO.isDef() &&
> + MRegisterInfo::isVirtualRegister(MO.getReg()))
> + VRegDefs[MO.getReg()] = &MI;
> + }
> + }
> + }
> +}
> +
> +/// HoistRegion - Walk the specified region of the CFG (defined by
> all blocks
> +/// dominated by the specified block, and that are in the current
> loop) in depth
> +/// first order w.r.t the DominatorTree. This allows us to visit
> definitions
> +/// before uses, allowing us to hoist a loop body in one pass
> without iteration.
> +///
> +void MachineLICM::HoistRegion(MachineDomTreeNode *N) {
> + assert(N != 0 && "Null dominator tree node?");
> + MachineBasicBlock *BB = N->getBlock();
> +
> + // If this subregion is not in the top level loop at all, exit.
> + if (!CurLoop->contains(BB)) return;
> +
> + // Only need to process the contents of this block if it is not
> part of a
> + // subloop (which would already have been processed).
> + if (!isInSubLoop(BB))
Like I said earlier, I think this check can be eliminated if we visit
the inner most loops first (and perhaps keep track which BB has been
processed?)
>
> + for (MachineBasicBlock::iterator
> + I = BB->begin(), E = BB->end(); I != E; ) {
> + MachineInstr &MI = *I++;
> +
> + // Try hoisting the instruction out of the loop. We can only
> do this if
> + // all of the operands of the instruction are loop invariant
> and if it is
> + // safe to hoist the instruction.
> + if (Hoist(MI))
> + // Hoisting was successful! Remove bothersome instruction
> now.
> + MI.getParent()->remove(&MI);
Why not have Hoist remove the MI?
>
> + }
> +
> + const std::vector<MachineDomTreeNode*> &Children = N-
> >getChildren();
> +
> + for (unsigned I = 0, E = Children.size(); I != E; ++I)
> + HoistRegion(Children[I]);
> +}
> +
> +/// isLoopInvariantInst - Returns true if the instruction is loop
> +/// invariant. I.e., all virtual register operands are defined
> outside of the
> +/// loop, physical registers aren't accessed (explicitly or
> implicitly), and the
> +/// instruction is hoistable.
> +///
> +bool MachineLICM::isLoopInvariantInst(MachineInstr &I) {
> + const TargetInstrDescriptor *TID = I.getInstrDescriptor();
> +
> + // Don't hoist if this instruction implicitly reads physical
> registers or
> + // doesn't take any operands.
> + if (TID->ImplicitUses || !I.getNumOperands()) return false;
The comment is wrong. It's ok to lift something that has no uses but
produces result(s), right? "doesn't take any operands" to me means
something that doesn't have any uses.
>
> +
> + if (!CanHoistInst(I)) return false;
Perhaps fold the earlier check into CanHoistInst()?
>
> +
> + // The instruction is loop invariant if all of its operands are
> loop-invariant
> + for (unsigned i = 0, e = I.getNumOperands(); i != e; ++i) {
> + const MachineOperand &MO = I.getOperand(i);
> +
> + if (!MO.isRegister() || !MO.isUse())
> + continue;
> +
> + unsigned Reg = MO.getReg();
> +
> + // Don't hoist instructions that access physical registers.
> + if (!MRegisterInfo::isVirtualRegister(Reg))
> + return false;
> +
> + assert(VRegDefs[Reg] && "Machine instr not mapped for this
> vreg?!");
> +
> + // If the loop contains the definition of an operand, then the
> instruction
> + // isn't loop invariant.
> + if (CurLoop->contains(VRegDefs[Reg]->getParent()))
> + return false;
Hmmm. I am not sure about this. What if the definition is in the
preheader? Does contains() returns true? Chris, Owen?
Also, I fear this might be slow. When you are creating the VReg -> MI
map, perhaps you should create a VReg -> loop map as well?
>
> + }
> +
> + // If we got this far, the instruction is loop invariant!
> + return true;
> +}
> +
> +/// Hoist - When an instruction is found to only use loop invariant
> operands
> +/// that is safe to hoist, this instruction is called to do the
> dirty work.
> +///
> +bool MachineLICM::Hoist(MachineInstr &MI) {
> + if (!isLoopInvariantInst(MI)) return false;
> +
> + std::vector<MachineBasicBlock*> Preds;
> +
> + // Non-back-edge predecessors.
> + FindPredecessors(Preds);
Consider caching some of these info?
>
> + if (Preds.empty()) return false;
>
> +
> + // Check that the predecessors are qualified to take the hoisted
> + // instruction. I.e., there is only one edge from each
> predecessor, and it's
> + // to the loop header.
> + for (std::vector<MachineBasicBlock*>::iterator
> + I = Preds.begin(), E = Preds.end(); I != E; ++I) {
> + MachineBasicBlock *MBB = *I;
> +
> + // FIXME: We are assuming at first that the basic blocks coming
> into this
> + // loop have only one successor each. This isn't the case in
> general because
> + // we haven't broken critical edges or added preheaders.
> + if (MBB->succ_size() != 1) return false;
> + assert(*MBB->succ_begin() == CurLoop->getHeader() &&
> + "The predecessor doesn't feed directly into the loop
> header!");
> + }
Are we going to create a preheader to hoist LICM to? Then we won't
need to do all these checking? I am entirely sure about it. Someone
please chime in.
>
> +
> + // Now move the instructions to the predecessors.
> + for (std::vector<MachineBasicBlock*>::iterator
> + I = Preds.begin(), E = Preds.end(); I != E; ++I)
> + MoveInstToBlock(*I, MI.clone());
This will create multiple copies, it seems bad. Again, this should be
fixable with a loop preheader?
Very nice first cut!
Evan
>
> +
> + Changed = true;
> + return true;
> +}
>
> Modified: llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.td?rev=44687&r1=44686&r2=44687&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.td (original)
> +++ llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.td Fri Dec 7
> 15:42:31 2007
> @@ -684,12 +684,15 @@
> def SUBFIC : DForm_2< 8, (outs GPRC:$rD), (ins GPRC:$rA, s16imm:$imm),
> "subfic $rD, $rA, $imm", IntGeneral,
> [(set GPRC:$rD, (subc immSExt16:$imm, GPRC:
> $rA))]>;
> -def LI : DForm_2_r0<14, (outs GPRC:$rD), (ins symbolLo:$imm),
> - "li $rD, $imm", IntGeneral,
> - [(set GPRC:$rD, immSExt16:$imm)]>;
> -def LIS : DForm_2_r0<15, (outs GPRC:$rD), (ins symbolHi:$imm),
> - "lis $rD, $imm", IntGeneral,
> - [(set GPRC:$rD, imm16ShiftedSExt:$imm)]>;
> +
> +let isReMaterializable = 1 in {
> + def LI : DForm_2_r0<14, (outs GPRC:$rD), (ins symbolLo:$imm),
> + "li $rD, $imm", IntGeneral,
> + [(set GPRC:$rD, immSExt16:$imm)]>;
> + def LIS : DForm_2_r0<15, (outs GPRC:$rD), (ins symbolHi:$imm),
> + "lis $rD, $imm", IntGeneral,
> + [(set GPRC:$rD, imm16ShiftedSExt:$imm)]>;
> +}
> }
>
> let PPC970_Unit = 1 in { // FXU Operations.
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list