From: Peizhao Ou Date: Thu, 16 Nov 2017 00:00:46 +0000 (-0800) Subject: Fixes untainted branch that is immediately after relaxed loads X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=commitdiff_plain;h=59278dfa2316824957b2633f5030103965030643 Fixes untainted branch that is immediately after relaxed loads --- diff --git a/lib/CodeGen/AtomicExpandPass.cpp b/lib/CodeGen/AtomicExpandPass.cpp index c4eff8823a1..faf951985a7 100644 --- a/lib/CodeGen/AtomicExpandPass.cpp +++ b/lib/CodeGen/AtomicExpandPass.cpp @@ -504,13 +504,17 @@ bool taintConditionalBranch(BranchInst* BI, Value* DepVal) { Builder.CreateTrunc(AndDep, IntegerType::get(DepVal->getContext(), 1)); auto* OrCond = Builder.CreateOr(TruncAndDep, Cond); BI->setOperand(0, OrCond); + + // Debug output. + DEBUG(dbgs() << "\tTainted branch condition:\n" << *BI->getParent()); + return true; } // XXX-update: For a relaxed load 'LI', find the first immediate atomic store or -// the first conditional branch. Returns itself if 'LI' can be left as is; -// returns nullptr if there's no such immediately following store/branch -// instructions, which we can only enforce the load with 'acquire'. +// 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'. Instruction* findFirstStoreCondBranchInst(LoadInst* LI) { // In some situations, relaxed loads can be left as is: // 1. The relaxed load is used to calculate the address of the immediate @@ -522,6 +526,8 @@ Instruction* findFirstStoreCondBranchInst(LoadInst* LI) { // 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. auto* BB = LI->getParent(); auto BE = BB->end(); @@ -550,35 +556,37 @@ Instruction* findFirstStoreCondBranchInst(LoadInst* LI) { } } if (BBI == BE) { - return LI; + return nullptr; } } } -void taintMonotonicLoads(const SmallVector& MonotonicLoadInsts) { +// XXX-comment: Returns whether the code has been changed. +bool taintMonotonicLoads(const SmallVector& MonotonicLoadInsts) { + bool Changed = false; for (auto* LI : MonotonicLoadInsts) { auto* FirstInst = findFirstStoreCondBranchInst(LI); if (FirstInst == nullptr) { - // No need to worry about the relaxed load. - continue; - } - if (FirstInst == LI) { // We don't seem to be able to taint a following store/conditional branch // instruction. Simply make it acquire. + DEBUG(dbgs() << "[RelaxedLoad]: Transformed to acquire load\n" + << *LI << "\n"); LI->setOrdering(Acquire); + Changed = true; continue; } // Taint 'FirstInst', which could be a store or a condition branch // instruction. if (FirstInst->getOpcode() == Instruction::Store) { - taintStoreAddress(dyn_cast(FirstInst), LI); + Changed |= taintStoreAddress(dyn_cast(FirstInst), LI); } else if (FirstInst->getOpcode() == Instruction::Br) { - taintConditionalBranch(dyn_cast(FirstInst), LI); + Changed |= taintConditionalBranch(dyn_cast(FirstInst), LI); } else { assert(false && "findFirstStoreCondBranchInst() should return a " "store/condition branch instruction"); } } + return Changed; } /**** Implementations of public methods for dependence tainting ****/ @@ -977,7 +985,7 @@ bool AtomicExpand::runOnFunction(Function &F) { } } - taintMonotonicLoads(MonotonicLoadInsts); + MadeChange |= taintMonotonicLoads(MonotonicLoadInsts); return MadeChange; } diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 86732bb6ffc..bdcf5a2a89e 100644 --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -3100,9 +3100,19 @@ SDValue DAGCombiner::visitAND(SDNode *N) { ConstantSDNode *N1C = dyn_cast(N1); // XXX-disabled: (and x, 0) should not be folded. + // (and (and x, 0), y) shouldn't either. if (!N0C && N1C->isNullValue()) { return SDValue(); } + if (!N0C) { + if (N0.getOpcode() == ISD::AND) { + auto* N01 = N0.getOperand(1).getNode(); + auto* N01C = dyn_cast(N01); + if (N01C && N01C->isNullValue()) { + return SDValue(); + } + } + } if (N0C && N1C && !N1C->isOpaque()) return DAG.FoldConstantArithmetic(ISD::AND, SDLoc(N), VT, N0C, N1C);