From: Dan Gohman Date: Fri, 30 Jan 2009 02:49:14 +0000 (+0000) Subject: Fix a post-RA scheduling dependency bug. X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=3311a1f8f0d8a2c6d940802bbb95eba0b801a615;p=oota-llvm.git Fix a post-RA scheduling dependency bug. If a MachineInstr doesn't have a memoperand but has an opcode that is known to load or store, assume its memory reference may alias *anything*, including stack slots which the compiler completely controls. To partially compensate for this, teach the ScheduleDAG building code to do basic getUnderlyingValue analysis. This greatly reduces the number of instructions that require restrictive dependencies. This code will need to be revisited when we start doing real alias analysis, but it should suffice for now. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@63370 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/Analysis/AliasAnalysis.h b/include/llvm/Analysis/AliasAnalysis.h index c705bccca4d..2beb8fb0bf1 100644 --- a/include/llvm/Analysis/AliasAnalysis.h +++ b/include/llvm/Analysis/AliasAnalysis.h @@ -345,6 +345,11 @@ public: } }; +/// isIdentifiedObject - Return true if this pointer refers to a distinct and +/// identifiable object. +/// +bool isIdentifiedObject(const Value *V); + } // End llvm namespace // Because of the way .a files work, we must force the BasicAA implementation to diff --git a/lib/Analysis/BasicAliasAnalysis.cpp b/lib/Analysis/BasicAliasAnalysis.cpp index 82514400b82..1002868e84c 100644 --- a/lib/Analysis/BasicAliasAnalysis.cpp +++ b/lib/Analysis/BasicAliasAnalysis.cpp @@ -80,7 +80,7 @@ static bool isNoAliasCall(const Value *V) { /// ByVal and NoAlias Arguments /// NoAlias returns /// -static bool isIdentifiedObject(const Value *V) { +bool llvm::isIdentifiedObject(const Value *V) { if (isa(V) || isa(V) || isNoAliasCall(V)) return true; if (const Argument *A = dyn_cast(V)) diff --git a/lib/CodeGen/ScheduleDAGInstrs.cpp b/lib/CodeGen/ScheduleDAGInstrs.cpp index 0ffdd05c2ee..9541de64242 100644 --- a/lib/CodeGen/ScheduleDAGInstrs.cpp +++ b/lib/CodeGen/ScheduleDAGInstrs.cpp @@ -13,6 +13,7 @@ //===----------------------------------------------------------------------===// #define DEBUG_TYPE "sched-instrs" +#include "llvm/Analysis/AliasAnalysis.h" #include "llvm/CodeGen/MachineDominators.h" #include "llvm/CodeGen/MachineFunctionPass.h" #include "llvm/CodeGen/MachineLoopInfo.h" @@ -95,6 +96,82 @@ ScheduleDAGInstrs::ScheduleDAGInstrs(MachineFunction &mf, const MachineDominatorTree &mdt) : ScheduleDAG(mf), MLI(mli), MDT(mdt) {} +/// getOpcode - If this is an Instruction or a ConstantExpr, return the +/// opcode value. Otherwise return UserOp1. +static unsigned getOpcode(const Value *V) { + if (const Instruction *I = dyn_cast(V)) + return I->getOpcode(); + if (const ConstantExpr *CE = dyn_cast(V)) + return CE->getOpcode(); + // Use UserOp1 to mean there's no opcode. + return Instruction::UserOp1; +} + +/// getUnderlyingObjectFromInt - This is the function that does the work of +/// looking through basic ptrtoint+arithmetic+inttoptr sequences. +static const Value *getUnderlyingObjectFromInt(const Value *V) { + do { + if (const User *U = dyn_cast(V)) { + // If we find a ptrtoint, we can transfer control back to the + // regular getUnderlyingObjectFromInt. + if (getOpcode(U) == Instruction::PtrToInt) + return U->getOperand(0); + // If we find an add of a constant or a multiplied value, it's + // likely that the other operand will lead us to the base + // object. We don't have to worry about the case where the + // object address is somehow being computed bt the multiply, + // because our callers only care when the result is an + // identifibale object. + if (getOpcode(U) != Instruction::Add || + (!isa(U->getOperand(1)) && + getOpcode(U->getOperand(1)) != Instruction::Mul)) + return V; + V = U->getOperand(0); + } else { + return V; + } + assert(isa(V->getType()) && "Unexpected operand type!"); + } while (1); +} + +/// getUnderlyingObject - This is a wrapper around Value::getUnderlyingObject +/// and adds support for basic ptrtoint+arithmetic+inttoptr sequences. +static const Value *getUnderlyingObject(const Value *V) { + // First just call Value::getUnderlyingObject to let it do what it does. + do { + V = V->getUnderlyingObject(); + // If it found an inttoptr, use special code to continue climing. + if (getOpcode(V) != Instruction::IntToPtr) + break; + const Value *O = getUnderlyingObjectFromInt(cast(V)->getOperand(0)); + // If that succeeded in finding a pointer, continue the search. + if (!isa(O->getType())) + break; + V = O; + } while (1); + return V; +} + +/// getUnderlyingObjectForInstr - If this machine instr has memory reference +/// information and it can be tracked to a normal reference to a known +/// object, return the Value for that object. Otherwise return null. +static const Value *getUnderlyingObjectForInstr(const MachineInstr *MI) { + if (!MI->hasOneMemOperand() || + !MI->memoperands_begin()->getValue() || + MI->memoperands_begin()->isVolatile()) + return 0; + + const Value *V = MI->memoperands_begin()->getValue(); + if (!V) + return 0; + + V = getUnderlyingObject(V); + if (!isa(V) && !isIdentifiedObject(V)) + return 0; + + return V; +} + void ScheduleDAGInstrs::BuildSchedGraph() { SUnits.reserve(BB->size()); @@ -313,12 +390,8 @@ void ScheduleDAGInstrs::BuildSchedGraph() { // Unknown memory accesses. Assume the worst. ChainMMO = 0; } else if (TID.mayStore()) { - if (MI->hasOneMemOperand() && - MI->memoperands_begin()->getValue() && - !MI->memoperands_begin()->isVolatile() && - isa(MI->memoperands_begin()->getValue())) { + if (const Value *V = getUnderlyingObjectForInstr(MI)) { // A store to a specific PseudoSourceValue. Add precise dependencies. - const Value *V = MI->memoperands_begin()->getValue(); // Handle the def in MemDefs, if there is one. std::map::iterator I = MemDefs.find(V); if (I != MemDefs.end()) { @@ -337,6 +410,10 @@ void ScheduleDAGInstrs::BuildSchedGraph() { /*isNormalMemory=*/true)); J->second.clear(); } + // Add dependencies from all the PendingLoads, since without + // memoperands we must assume they alias anything. + for (unsigned k = 0, m = PendingLoads.size(); k != m; ++k) + PendingLoads[k]->addPred(SDep(SU, SDep::Order, SU->Latency)); // Add a general dependence too, if needed. if (Chain) Chain->addPred(SDep(SU, SDep::Order, SU->Latency)); @@ -346,12 +423,8 @@ void ScheduleDAGInstrs::BuildSchedGraph() { } else if (TID.mayLoad()) { if (TII->isInvariantLoad(MI)) { // Invariant load, no chain dependencies needed! - } else if (MI->hasOneMemOperand() && - MI->memoperands_begin()->getValue() && - !MI->memoperands_begin()->isVolatile() && - isa(MI->memoperands_begin()->getValue())) { + } else if (const Value *V = getUnderlyingObjectForInstr(MI)) { // A load from a specific PseudoSourceValue. Add precise dependencies. - const Value *V = MI->memoperands_begin()->getValue(); std::map::iterator I = MemDefs.find(V); if (I != MemDefs.end()) I->second->addPred(SDep(SU, SDep::Order, SU->Latency, /*Reg=*/0, @@ -367,9 +440,15 @@ void ScheduleDAGInstrs::BuildSchedGraph() { // cases where memoperand information is unavailable. goto new_chain; } else { - // A normal load. Just depend on the general chain. + // A normal load. Depend on the general chain, as well as on + // all stores. In the absense of MachineMemOperand information, + // we can't even assume that the load doesn't alias well-behaved + // memory locations. if (Chain) Chain->addPred(SDep(SU, SDep::Order, SU->Latency)); + for (std::map::iterator I = MemDefs.begin(), + E = MemDefs.end(); I != E; ++I) + I->second->addPred(SDep(SU, SDep::Order, SU->Latency)); PendingLoads.push_back(SU); } }