From: Peizhao Ou Date: Fri, 13 Apr 2018 05:40:23 +0000 (-0700) Subject: Taints the first upcoming load/store or adds bogus cond branch after relaxed loads X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=73a4da7d234060381bdb4cfa51618d6ae7ad4ac2;p=oota-llvm.git Taints the first upcoming load/store or adds bogus cond branch after relaxed loads --- diff --git a/lib/CodeGen/CodeGenPrepare.cpp b/lib/CodeGen/CodeGenPrepare.cpp index c9d22a29865..8b9111cafda 100644 --- a/lib/CodeGen/CodeGenPrepare.cpp +++ b/lib/CodeGen/CodeGenPrepare.cpp @@ -221,6 +221,7 @@ FunctionPass *llvm::createCodeGenPreparePass(const TargetMachine *TM) { namespace { bool StoreAddressDependOnValue(StoreInst* SI, Value* DepVal); +bool LoadAddressDependOnValue(LoadInst* SI, Value* DepVal); Value* GetUntaintedAddress(Value* CurrentAddress); // The depth we trace down a variable to look for its dependence set. @@ -624,6 +625,98 @@ bool taintStoreAddress(StoreInst* SI, Value* DepVal) { return true; } + +// This function actually taints 'DepVal' to the address to 'LI'. If the +// address +// of 'LI' already depends on whatever 'DepVal' depends on, this function +// doesn't do anything and returns false. Otherwise, returns true. +// +// For example, +// %0 = load i32, i32* @y, align 4, !tbaa !1 +// %cmp = icmp ne i32 %0, 42 // <== this is like the condition +// %1 = sext i1 %cmp to i32 +// %2 = ptrtoint i32* @x to i32 +// %3 = and i32 %1, 0 +// %4 = or i32 %3, %2 +// %5 = inttoptr i32 %4 to i32* +// %1 = i32 1, i32* %5, align 4 +bool taintLoadAddress(LoadInst* LI, Value* DepVal) { + // Set the insertion point right after the 'DepVal'. + Instruction* Inst = nullptr; + IRBuilder Builder(LI); + BasicBlock* BB = LI->getParent(); + Value* Address = LI->getPointerOperand(); + Type* TargetIntegerType = + IntegerType::get(Address->getContext(), + BB->getModule()->getDataLayout().getPointerSizeInBits()); + + // Does LI's address already depends on whatever 'DepVal' depends on? + if (LoadAddressDependOnValue(LI, DepVal)) { + return false; + } + + // Figure out if there's a root variable 'DepVal' depends on. For example, we + // can extract "getelementptr inbounds %struct, %struct* %0, i64 0, i32 123" + // to be "%struct* %0" since all other operands are constant. + auto* RootVal = getRootDependence(DepVal); + auto* RootInst = dyn_cast(RootVal); + auto* DepValInst = dyn_cast(DepVal); + if (RootInst && DepValInst && + RootInst->getParent() == DepValInst->getParent()) { + DepVal = RootVal; + } + + // Is this already a dependence-tainted load? + Value* OldDep = getDependence(Address); + if (OldDep) { + // The address of 'LI' has already been tainted. Just need to absorb the + // DepVal to the existing dependence in the address of LI. + Instruction* AndDep = getAndDependence(Address); + IRBuilder Builder(AndDep); + Value* NewDep = nullptr; + if (DepVal->getType() == AndDep->getType()) { + NewDep = Builder.CreateAnd(OldDep, DepVal); + } else { + NewDep = Builder.CreateAnd( + OldDep, createCast(Builder, DepVal, TargetIntegerType)); + } + + auto* NewDepInst = dyn_cast(NewDep); + + // Use the new AND instruction as the dependence + AndDep->setOperand(0, NewDep); + return true; + } + + // LI's address has not been tainted. Now taint it with 'DepVal'. + Value* CastDepToInt = createCast(Builder, DepVal, TargetIntegerType); + Value* PtrToIntCast = Builder.CreatePtrToInt(Address, TargetIntegerType); + Value* AndDepVal = + Builder.CreateAnd(CastDepToInt, ConstantInt::get(TargetIntegerType, 0)); + auto AndInst = dyn_cast(AndDepVal); + // XXX-comment: The original IR InstCombiner would change our and instruction + // to a select and then the back end optimize the condition out. We attach a + // flag to instructions and set it here to inform the InstCombiner to not to + // touch this and instruction at all. + Value* OrAddr = Builder.CreateOr(AndDepVal, PtrToIntCast); + Value* NewAddr = Builder.CreateIntToPtr(OrAddr, Address->getType()); + + DEBUG(dbgs() << "[taintLoadAddress]\n" + << "Original load: " << *LI << '\n'); + LI->setOperand(0, NewAddr); + + // Debug output. + DEBUG(dbgs() << "\tTargetIntegerType: " << *TargetIntegerType << '\n' + << "\tCast dependence value to integer: " << *CastDepToInt + << '\n' + << "\tCast address to integer: " << *PtrToIntCast << '\n' + << "\tAnd dependence value: " << *AndDepVal << '\n' + << "\tOr address: " << *OrAddr << '\n' + << "\tCast or instruction to address: " << *NewAddr << "\n\n"); + + return true; +} + // Looks for the previous store in the if block --- 'BrBB', which makes the // speculative store 'StoreToHoist' safe. Value* getSpeculativeStoreInPrevBB(StoreInst* StoreToHoist, BasicBlock* BrBB) { @@ -681,7 +774,7 @@ bool ConditionalBranchDependsOnValue(BranchInst* BI, Value* DepVal) { return dependenceSetInclusion(Cond, DepVal); } -// XXX-update: For a relaxed load 'LI', find the first immediate atomic store or +// XXX-update: For a relaxed load 'LI', find the first immediate store or // the first conditional branch. Returns nullptr if there's no such immediately // following store/branch instructions, which we can only enforce the load with // 'acquire'. 'ChainedBB' contains all the blocks chained together with @@ -712,8 +805,72 @@ Instruction* findFirstStoreCondBranchInst(LoadInst* LI, Vector* ChainedBB) { for (; BBI != BE; BBI++) { Instruction* Inst = &*BBI; IntrinsicInst* II = dyn_cast(&*BBI); - if (II && II->getIntrinsicID() == Intrinsic::aarch64_stlxr) { - return II; + if (II) { + if (II->getIntrinsicID() == Intrinsic::aarch64_stlxr) { + return II; + } + } else if (Inst->getOpcode() == Instruction::Store) { + return Inst; + } else if (Inst->getOpcode() == Instruction::Br) { + auto* BrInst = dyn_cast(Inst); + if (BrInst->isConditional()) { + return Inst; + } else { + // Reinitialize iterators with the destination of the unconditional + // branch. + BB = BrInst->getSuccessor(0); + ChainedBB->push_back(BB); + BBI = BB->begin(); + BE = BB->end(); + break; + } + } + } + if (BBI == BE) { + return nullptr; + } + } +} + +// XXX-update: For a relaxed load 'LI', find the first immediate load/store or +// the first conditional branch. Returns nullptr if there's no such immediately +// following store/branch instructions, which we can only enforce the load with +// 'acquire'. 'ChainedBB' contains all the blocks chained together with +// unconditional branches from 'BB' to the block with the first store/cond +// branch. +template +Instruction* findFirstLoadStoreCondBranchInst(LoadInst* LI, Vector* ChainedBB) { + // In some situations, relaxed loads can be left as is: + // 1. The relaxed load is used to calculate the address of the immediate + // following store; + // 2. The relaxed load is used as a condition in the immediate following + // condition, and there are no stores in between. This is actually quite + // common. E.g., + // int r1 = x.load(relaxed); + // if (r1 != 0) { + // y.store(1, relaxed); + // } + // However, in this function, we don't deal with them directly. Instead, we + // just find the immediate following store/condition branch and return it. + + assert(ChainedBB != nullptr && "Chained BB should not be nullptr"); + auto* BB = LI->getParent(); + ChainedBB->push_back(BB); + auto BE = BB->end(); + auto BBI = BasicBlock::iterator(LI); + BBI++; + while (true) { + for (; BBI != BE; BBI++) { + Instruction* Inst = &*BBI; + IntrinsicInst* II = dyn_cast(&*BBI); + if (II) { + if (II->getIntrinsicID() == Intrinsic::aarch64_stlxr || + II->getIntrinsicID() == Intrinsic::aarch64_ldxr) { + return II; + } + } else if (Inst->getOpcode() == Instruction::Load) { + // FIXME: Should find the last load or the first store/cond branch! + return Inst; } else if (Inst->getOpcode() == Instruction::Store) { return Inst; } else if (Inst->getOpcode() == Instruction::Br) { @@ -935,6 +1092,50 @@ Instruction* findMostRecentDependenceUsage(LoadInst* LI, Instruction* LaterInst, return usage_inst; } +// XXX-comment: For relaxed load 'LI', and the first upcoming store/conditional +// branch instruction 'FirstInst', returns whether there are any intermediate +// instructions I (including 'FirstInst') that satisfy: +// 1. I is a load/store, and its address depends on 'LI'. +// 2. I is a conditional branch whose condition depends on 'LI'. +// Note that 'LI' and 'FirstInst' can be in different basic blocks, but LI's +// basic block can unconditionally jumps (by steps) to FirstInst's block. +bool NeedExtraConstraints(LoadInst* LI, Instruction* FirstInst) { + if (!FirstInst) { + return true; + } + auto* BB = LI->getParent(); + auto BBI = LI->getIterator(); + BBI++; + while (true) { + auto* I = &*BBI; + BBI++; + BranchInst *BI = dyn_cast(I); + if (BI && BI->isUnconditional()) { + BasicBlock *DestBB = BI->getSuccessor(0); + BBI = DestBB->begin(); + continue; + } + + if (I->getOpcode() == Instruction::Store) { + if (StoreAddressDependOnValue(dyn_cast(I), LI)) { + return false; + } + } else if (I->getOpcode() == Instruction::Load) { + if (LoadAddressDependOnValue(dyn_cast(I), LI)) { + return false; + } + } else if (I->getOpcode() == Instruction::Br) { + if (ConditionalBranchDependsOnValue(dyn_cast(I), LI)) { + return false; + } + } + if (I == FirstInst) { + return true; + } + } + return true; +} + // XXX-comment: Returns whether the code has been changed. bool AddFakeConditionalBranchAfterMonotonicLoads( SmallSet& MonotonicLoadInsts, DominatorTree* DT) { @@ -943,33 +1144,20 @@ bool AddFakeConditionalBranchAfterMonotonicLoads( auto* LI = *MonotonicLoadInsts.begin(); MonotonicLoadInsts.erase(LI); SmallVector ChainedBB; - auto* FirstInst = findFirstStoreCondBranchInst(LI, &ChainedBB); - if (FirstInst != nullptr) { - if (FirstInst->getOpcode() == Instruction::Store) { - if (StoreAddressDependOnValue(dyn_cast(FirstInst), LI)) { - continue; - } - } else if (FirstInst->getOpcode() == Instruction::Br) { - if (ConditionalBranchDependsOnValue(dyn_cast(FirstInst), - LI)) { - continue; - } - } else { - IntrinsicInst* II = dyn_cast(FirstInst); - if (!II || II->getIntrinsicID() != Intrinsic::aarch64_stlxr) { - dbgs() << "FirstInst=" << *FirstInst << "\n"; - assert(false && "findFirstStoreCondBranchInst() should return a " - "store/condition branch instruction"); - } - } + auto* FirstInst = findFirstLoadStoreCondBranchInst(LI, &ChainedBB); + if (!NeedExtraConstraints(LI, FirstInst)) { + // 'LI' doesn't need extra load-store constraints. + continue; } // We really need to process the relaxed load now. Note that if the next // instruction is a RMW, it will be transformed into a control block, so we // can safely only taint upcoming store instructions. + LoadInst* LdInst = nullptr; StoreInst* SI = nullptr; IntrinsicInst* II = nullptr; if (FirstInst) { + LdInst = dyn_cast(FirstInst); SI = dyn_cast(FirstInst); II = dyn_cast(FirstInst); } @@ -989,6 +1177,22 @@ bool AddFakeConditionalBranchAfterMonotonicLoads( Changed |= taintStoreAddress(SI, Inst); } } + } else if (FirstInst && LdInst) { + // For immediately coming loads, taint the address of the load. + if (FirstInst->getParent() == LI->getParent() || + DT->dominates(LI, FirstInst)) { + Changed != taintLoadAddress(LdInst, LI); + Changed = true; + } else { + auto* Inst = + findMostRecentDependenceUsage(LI, FirstInst, &ChainedBB, DT); + if (!Inst) { + LI->setOrdering(Acquire); + Changed = true; + } else { + Changed |= taintLoadAddress(LdInst, Inst); + } + } } else { // No upcoming branch if (!FirstInst) { @@ -1260,6 +1464,10 @@ bool StoreAddressDependOnValue(StoreInst* SI, Value* DepVal) { return dependenceSetInclusion(SI->getPointerOperand(), DepVal); } +bool LoadAddressDependOnValue(LoadInst* LI, Value* DepVal) { + return dependenceSetInclusion(LI->getPointerOperand(), DepVal); +} + bool StoreDependOnValue(StoreInst* SI, Value* Dep) { return dependenceSetInclusion(SI, Dep); }