[llvm] r225481 - [Refactor] Have getNonLocalPointerDependency take the query instruction
Philip Reames
listmail at philipreames.com
Thu Jan 8 16:56:31 PST 2015
p.s. It looks like I broke the non-assert builds with a variable which
was only used in an assert. My follow up commit 225483 has already
modified this code and should unbreak the build.
> + if (LoadInst *LI = dyn_cast<LoadInst>(QueryInst)) {
> + assert(!LI->isVolatile());
For the record, I find this particular warning/error to more frustrating
than useful. Having a variable that used only in an assert is a
reasonable coding practice and should not result in a broken build.
Philip
On 01/08/2015 04:31 PM, Philip Reames wrote:
> The review for this was http://reviews.llvm.org/D6886.
>
> Sorry I forgot to add that to the commit message.
>
> On 01/08/2015 04:04 PM, Philip Reames wrote:
>> Author: reames
>> Date: Thu Jan 8 18:04:22 2015
>> New Revision: 225481
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=225481&view=rev
>> Log:
>> [Refactor] Have getNonLocalPointerDependency take the query instruction
>>
>> Previously, MemoryDependenceAnalysis::getNonLocalPointerDependency
>> was taking a list of properties about the instruction being queried.
>> Since I'm about to need one more property to be passed down through
>> the infrastructure - I need to know a query instruction is
>> non-volatile in an inner helper - fix the interface once and for all.
>>
>> I also added some assertions and behaviour clarifications around
>> volatile and ordered field accesses. At the moment, this is mostly to
>> document expected behaviour. The only non-standard instructions which
>> can currently reach this are atomic, but unordered, loads and stores.
>> Neither ordered or volatile accesses can reach here.
>>
>> The call in GVN is protected by an isSimple check when it first
>> considers the load. The calls in MemDepPrinter are protected by
>> isUnordered checks. Both utilities also check isVolatile for loads
>> and stores.
>>
>>
>> Modified:
>> llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h
>> llvm/trunk/lib/Analysis/MemDepPrinter.cpp
>> llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp
>> llvm/trunk/lib/Transforms/Scalar/GVN.cpp
>>
>> Modified: llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h?rev=225481&r1=225480&r2=225481&view=diff
>> ==============================================================================
>>
>> --- llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h
>> (original)
>> +++ llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h Thu
>> Jan 8 18:04:22 2015
>> @@ -366,12 +366,13 @@ namespace llvm {
>> /// getNonLocalPointerDependency - Perform a full
>> dependency query for an
>> - /// access to the specified (non-volatile) memory location,
>> returning the
>> - /// set of instructions that either define or clobber the value.
>> + /// access to the QueryInst's specified (non-volatile) memory
>> location,
>> + /// returning the set of instructions that either define or clobber
>> + /// the value.
>> ///
>> - /// This method assumes the pointer has a "NonLocal" dependency
>> within BB.
>> - void getNonLocalPointerDependency(const AliasAnalysis::Location
>> &Loc,
>> - bool isLoad, BasicBlock *BB,
>> + /// This method assumes the pointer has a "NonLocal" dependency
>> within
>> + /// QueryInst's parent basic block.
>> + void getNonLocalPointerDependency(Instruction *QueryInst,
>> SmallVectorImpl<NonLocalDepResult> &Result);
>> /// removeInstruction - Remove an instruction from the
>> dependence analysis,
>>
>> Modified: llvm/trunk/lib/Analysis/MemDepPrinter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemDepPrinter.cpp?rev=225481&r1=225480&r2=225481&view=diff
>> ==============================================================================
>>
>> --- llvm/trunk/lib/Analysis/MemDepPrinter.cpp (original)
>> +++ llvm/trunk/lib/Analysis/MemDepPrinter.cpp Thu Jan 8 18:04:22 2015
>> @@ -92,7 +92,6 @@ const char *const MemDepPrinter::DepType
>> bool MemDepPrinter::runOnFunction(Function &F) {
>> this->F = &F;
>> - AliasAnalysis &AA = getAnalysis<AliasAnalysis>();
>> MemoryDependenceAnalysis &MDA =
>> getAnalysis<MemoryDependenceAnalysis>();
>> // All this code uses non-const interfaces because MemDep is not
>> @@ -126,8 +125,7 @@ bool MemDepPrinter::runOnFunction(Functi
>> static_cast<BasicBlock *>(nullptr)));
>> continue;
>> }
>> - AliasAnalysis::Location Loc = AA.getLocation(LI);
>> - MDA.getNonLocalPointerDependency(Loc, true, LI->getParent(),
>> NLDI);
>> + MDA.getNonLocalPointerDependency(LI, NLDI);
>> } else if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
>> if (!SI->isUnordered()) {
>> // FIXME: Handle atomic/volatile stores.
>> @@ -135,11 +133,9 @@ bool MemDepPrinter::runOnFunction(Functi
>> static_cast<BasicBlock *>(nullptr)));
>> continue;
>> }
>> - AliasAnalysis::Location Loc = AA.getLocation(SI);
>> - MDA.getNonLocalPointerDependency(Loc, false,
>> SI->getParent(), NLDI);
>> + MDA.getNonLocalPointerDependency(SI, NLDI);
>> } else if (VAArgInst *VI = dyn_cast<VAArgInst>(Inst)) {
>> - AliasAnalysis::Location Loc = AA.getLocation(VI);
>> - MDA.getNonLocalPointerDependency(Loc, false,
>> VI->getParent(), NLDI);
>> + MDA.getNonLocalPointerDependency(VI, NLDI);
>> } else {
>> llvm_unreachable("Unknown memory instruction!");
>> }
>>
>> Modified: llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp?rev=225481&r1=225480&r2=225481&view=diff
>> ==============================================================================
>>
>> --- llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp (original)
>> +++ llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp Thu Jan 8
>> 18:04:22 2015
>> @@ -859,9 +859,53 @@ MemoryDependenceAnalysis::getNonLocalCal
>> /// own block.
>> ///
>> void MemoryDependenceAnalysis::
>> -getNonLocalPointerDependency(const AliasAnalysis::Location &Loc,
>> bool isLoad,
>> - BasicBlock *FromBB,
>> +getNonLocalPointerDependency(Instruction *QueryInst,
>> SmallVectorImpl<NonLocalDepResult> &Result) {
>> +
>> + auto getLocation = [](AliasAnalysis *AA, Instruction *Inst) {
>> + if (auto *I = dyn_cast<LoadInst>(Inst))
>> + return AA->getLocation(I);
>> + else if (auto *I = dyn_cast<StoreInst>(Inst))
>> + return AA->getLocation(I);
>> + else if (auto *I = dyn_cast<VAArgInst>(Inst))
>> + return AA->getLocation(I);
>> + else if (auto *I = dyn_cast<AtomicCmpXchgInst>(Inst))
>> + return AA->getLocation(I);
>> + else if (auto *I = dyn_cast<AtomicRMWInst>(Inst))
>> + return AA->getLocation(I);
>> + else
>> + llvm_unreachable("unsupported memory instruction");
>> + };
>> +
>> + const AliasAnalysis::Location Loc = getLocation(AA, QueryInst);
>> + bool isLoad = isa<LoadInst>(QueryInst);
>> + BasicBlock *FromBB = QueryInst->getParent();
>> + assert(FromBB);
>> +
>> + // This routine does not expect to deal with volatile
>> instructions. Doing so
>> + // would require piping through the QueryInst all the way through.
>> + // TODO: volatiles can't be elided, but they can be reordered with
>> other
>> + // non-volatile accesses.
>> + if (LoadInst *LI = dyn_cast<LoadInst>(QueryInst)) {
>> + assert(!LI->isVolatile());
>> + } else if (StoreInst *SI = dyn_cast<StoreInst>(QueryInst)) {
>> + assert(!SI->isVolatile());
>> + }
>> +
>> +
>> + // We currently give up on any instruction which is ordered, but
>> we do handle
>> + // atomic instructions which are unordered.
>> + // TODO: Handle ordered instructions
>> + auto isOrdered = [](Instruction *Inst) {
>> + if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
>> + return !LI->isUnordered();
>> + } else if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
>> + return !SI->isUnordered();
>> + }
>> + return false;
>> + };
>> + assert(!isOrdered(QueryInst) && "ordered instructions not expected");
>> +
>> assert(Loc.Ptr->getType()->isPointerTy() &&
>> "Can't get pointer deps of a non-pointer!");
>> Result.clear();
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=225481&r1=225480&r2=225481&view=diff
>> ==============================================================================
>>
>> --- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Thu Jan 8 18:04:22 2015
>> @@ -1707,8 +1707,7 @@ bool GVN::PerformLoadPRE(LoadInst *LI, A
>> bool GVN::processNonLocalLoad(LoadInst *LI) {
>> // Step 1: Find the non-local dependencies of the load.
>> LoadDepVect Deps;
>> - AliasAnalysis::Location Loc = VN.getAliasAnalysis()->getLocation(LI);
>> - MD->getNonLocalPointerDependency(Loc, true, LI->getParent(), Deps);
>> + MD->getNonLocalPointerDependency(LI, Deps);
>> // If we had to process more than one hundred blocks to find the
>> // dependencies, this load isn't worth worrying about. Optimizing
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> 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