From e8d5288528844ce3643b7debe333b749ef3b7c98 Mon Sep 17 00:00:00 2001 From: Peizhao Ou Date: Wed, 6 Dec 2017 16:45:15 -0800 Subject: [PATCH] Converts necessary relaxed loads to acquire loads --- include/llvm/CodeGen/MachineBasicBlock.h | 13 ---- include/llvm/IR/BasicBlock.h | 13 ---- lib/CodeGen/AtomicExpandPass.cpp | 60 +--------------- lib/CodeGen/BranchFolding.cpp | 6 -- lib/CodeGen/CodeGenPrepare.cpp | 48 ++----------- lib/CodeGen/MachineBasicBlock.cpp | 2 +- lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 72 ------------------- lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 4 -- lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | 44 ------------ lib/IR/BasicBlock.cpp | 3 +- lib/IR/Verifier.cpp | 7 -- lib/Target/AArch64/AArch64TargetMachine.cpp | 3 - 12 files changed, 9 insertions(+), 266 deletions(-) diff --git a/include/llvm/CodeGen/MachineBasicBlock.h b/include/llvm/CodeGen/MachineBasicBlock.h index fd786f68fb6..3d58c499823 100644 --- a/include/llvm/CodeGen/MachineBasicBlock.h +++ b/include/llvm/CodeGen/MachineBasicBlock.h @@ -124,10 +124,6 @@ private: /// is only computed once and is cached. mutable MCSymbol *CachedMCSymbol = nullptr; - // XXX-update: A flag that checks whether we can eliminate this machine basic - // block. - bool canEliminateMachineBB; - // Intrusive list support MachineBasicBlock() {} @@ -139,15 +135,6 @@ private: friend class MachineFunction; public: - // XXX-update: - void disableCanEliminateMachineBB() { - canEliminateMachineBB = false; - } - - bool getCanEliminateMachineBB() { - return canEliminateMachineBB; - } - /// Return the LLVM basic block that this instance corresponded to originally. /// Note that this may be NULL if this instance does not correspond directly /// to an LLVM basic block. diff --git a/include/llvm/IR/BasicBlock.h b/include/llvm/IR/BasicBlock.h index 94edb547f53..c6b54d308ce 100644 --- a/include/llvm/IR/BasicBlock.h +++ b/include/llvm/IR/BasicBlock.h @@ -59,9 +59,6 @@ private: InstListType InstList; Function *Parent; - // XXX-update: A flag that checks whether we can eliminate this block. - bool canEliminateBlock; - void setParent(Function *parent); friend class SymbolTableListTraits; @@ -77,16 +74,6 @@ private: Function *Parent = nullptr, BasicBlock *InsertBefore = nullptr); public: - // XXX-update: - void disableCanEliminateBlock() { - canEliminateBlock = false; - } - - bool getCanEliminateBlock() { - return canEliminateBlock; - } - - /// \brief Get the context in which this basic block lives. LLVMContext &getContext() const; diff --git a/lib/CodeGen/AtomicExpandPass.cpp b/lib/CodeGen/AtomicExpandPass.cpp index c8308afe9c1..d12fdb24698 100644 --- a/lib/CodeGen/AtomicExpandPass.cpp +++ b/lib/CodeGen/AtomicExpandPass.cpp @@ -15,11 +15,6 @@ // //===----------------------------------------------------------------------===// -#include "llvm/ADT/SetOperations.h" -#include "llvm/ADT/SetVector.h" -#include "llvm/ADT/SmallPtrSet.h" -#include "llvm/ADT/SmallVector.h" -#include "llvm/Analysis/MemoryLocation.h" #include "llvm/CodeGen/AtomicExpandUtils.h" #include "llvm/CodeGen/Passes.h" #include "llvm/IR/Function.h" @@ -28,13 +23,11 @@ #include "llvm/IR/Instructions.h" #include "llvm/IR/Intrinsics.h" #include "llvm/IR/Module.h" -#include "llvm/IR/NoFolder.h" #include "llvm/Support/Debug.h" #include "llvm/Support/raw_ostream.h" #include "llvm/Target/TargetLowering.h" #include "llvm/Target/TargetMachine.h" #include "llvm/Target/TargetSubtargetInfo.h" -#include "llvm/Transforms/Utils/BasicBlockUtils.h" using namespace llvm; @@ -89,60 +82,12 @@ bool AtomicExpand::runOnFunction(Function &F) { TLI = TM->getSubtargetImpl(F)->getTargetLowering(); SmallVector AtomicInsts; - SmallVector MonotonicLoadInsts; // Changing control-flow while iterating through it is a bad idea, so gather a // list of all atomic instructions before we start. for (inst_iterator I = inst_begin(F), E = inst_end(F); I != E; ++I) { - // XXX-update: For relaxed loads, change them to acquire. This includes - // relaxed loads, relaxed atomic RMW & relaxed atomic compare exchange. - if (I->isAtomic()) { - switch (I->getOpcode()) { - case Instruction::AtomicCmpXchg: { - // XXX-comment: AtomicCmpXchg in AArch64 will be translated to a - // conditional branch that contains the value of the load anyway, so - // we don't need to do anything. - /* - auto* CmpXchg = dyn_cast(&*I); - auto SuccOrdering = CmpXchg->getSuccessOrdering(); - if (SuccOrdering == Monotonic) { - CmpXchg->setSuccessOrdering(Acquire); - } else if (SuccOrdering == Release) { - CmpXchg->setSuccessOrdering(AcquireRelease); - } - */ - break; - } - case Instruction::AtomicRMW: { - // XXX-comment: Similar to AtomicCmpXchg. These instructions in - // AArch64 will be translated to a loop whose condition depends on the - // store status, which further depends on the load value. - /* - auto* RMW = dyn_cast(&*I); - if (RMW->getOrdering() == Monotonic) { - RMW->setOrdering(Acquire); - } - */ - break; - } - case Instruction::Load: { - auto* LI = dyn_cast(&*I); - if (LI->getOrdering() == Monotonic) { - /* - DEBUG(dbgs() << "Transforming relaxed loads to acquire loads: " - << *LI << '\n'); - LI->setOrdering(Acquire); - */ - MonotonicLoadInsts.push_back(LI); - } - break; - } - default: { - break; - } - } + if (I->isAtomic()) AtomicInsts.push_back(&*I); - } } bool MadeChange = false; @@ -159,7 +104,7 @@ bool AtomicExpand::runOnFunction(Function &F) { if (TLI->getInsertFencesForAtomic()) { if (LI && isAtLeastAcquire(LI->getOrdering())) { FenceOrdering = LI->getOrdering(); -// AddFakeConditionalBranch( + LI->setOrdering(Monotonic); IsStore = false; IsLoad = true; } else if (SI && isAtLeastRelease(SI->getOrdering())) { @@ -227,7 +172,6 @@ bool AtomicExpand::runOnFunction(Function &F) { MadeChange |= expandAtomicCmpXchg(CASI); } } - return MadeChange; } diff --git a/lib/CodeGen/BranchFolding.cpp b/lib/CodeGen/BranchFolding.cpp index a56fec468cc..df5cac5a9f7 100644 --- a/lib/CodeGen/BranchFolding.cpp +++ b/lib/CodeGen/BranchFolding.cpp @@ -1116,12 +1116,6 @@ bool BranchFolder::OptimizeBranches(MachineFunction &MF) { for (MachineFunction::iterator I = std::next(MF.begin()), E = MF.end(); I != E; ) { MachineBasicBlock *MBB = &*I++; - // XXX-disabled: Don't optimize blocks that contain intentionally added fake - // conditional branch. - if (!MBB->getCanEliminateMachineBB()) { - continue; - } - MadeChange |= OptimizeBlock(MBB); // If it is dead, remove it. diff --git a/lib/CodeGen/CodeGenPrepare.cpp b/lib/CodeGen/CodeGenPrepare.cpp index ac8fbbf9c76..85dbeb60561 100644 --- a/lib/CodeGen/CodeGenPrepare.cpp +++ b/lib/CodeGen/CodeGenPrepare.cpp @@ -773,9 +773,9 @@ void AddFakeConditionalBranch(Instruction* SplitInst, Value* Condition) { auto* TailBB = ThenBB->getSingleSuccessor(); assert(TailBB && "Tail block cannot be empty after basic block spliting"); - ThenBB->disableCanEliminateBlock(); - ThenBB->disableCanEliminateBlock(); - TailBB->disableCanEliminateBlock(); +// ThenBB->disableCanEliminateBlock(); +// ThenBB->disableCanEliminateBlock(); +// TailBB->disableCanEliminateBlock(); ThenBB->setName(BB->getName() + "Then.Fake"); ElseBB->setName(BB->getName() + "Else.Fake"); DEBUG(dbgs() << "Add fake conditional branch:\n" @@ -916,46 +916,8 @@ bool AddFakeConditionalBranchAfterMonotonicLoads( "store/condition branch instruction"); } } - - // We really need to process the relaxed load now. - StoreInst* SI = nullptr;; - if (FirstInst && (SI = dyn_cast(FirstInst))) { - // For immediately coming stores, taint the address of the store. - if (SI->getParent() == LI->getParent() || DT->dominates(LI, SI)) { - Changed |= taintStoreAddress(SI, LI); - } else { - auto* Inst = - findMostRecentDependenceUsage(LI, FirstInst, &ChainedBB, DT); - if (!Inst) { - LI->setOrdering(Acquire); - Changed = true; - } else { - Changed |= taintStoreAddress(SI, Inst); - } - } - } else { - // No upcoming branch - if (!FirstInst) { - TaintRelaxedLoads(LI); - Changed = true; - } else { - // For immediately coming branch, directly add a fake branch. - if (FirstInst->getParent() == LI->getParent() || - DT->dominates(LI, FirstInst)) { - TaintRelaxedLoads(LI); - Changed = true; - } else { - auto* Inst = - findMostRecentDependenceUsage(LI, FirstInst, &ChainedBB, DT); - if (Inst) { - TaintRelaxedLoads(Inst); - } else { - LI->setOrdering(Acquire); - } - Changed = true; - } - } - } + LI->setOrdering(Acquire); + Changed = true; } return Changed; } diff --git a/lib/CodeGen/MachineBasicBlock.cpp b/lib/CodeGen/MachineBasicBlock.cpp index 42190168742..85d544d9498 100644 --- a/lib/CodeGen/MachineBasicBlock.cpp +++ b/lib/CodeGen/MachineBasicBlock.cpp @@ -40,7 +40,7 @@ using namespace llvm; #define DEBUG_TYPE "codegen" MachineBasicBlock::MachineBasicBlock(MachineFunction &MF, const BasicBlock *B) - : BB(B), Number(-1), xParent(&MF), canEliminateMachineBB(true) { + : BB(B), Number(-1), xParent(&MF) { Insts.Parent = this; } diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index f119023d217..c741982bc08 100644 --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -924,62 +924,6 @@ CommitTargetLoweringOpt(const TargetLowering::TargetLoweringOpt &TLO) { bool DAGCombiner::SimplifyDemandedBits(SDValue Op, const APInt &Demanded) { TargetLowering::TargetLoweringOpt TLO(DAG, LegalTypes, LegalOperations); APInt KnownZero, KnownOne; - - // XXX-disabled: - auto Opcode = Op.getOpcode(); - if (Opcode == ISD::AND || Opcode == ISD::OR) { - auto* Op1 = Op.getOperand(0).getNode(); - auto* Op2 = Op.getOperand(1).getNode(); - auto* Op1C = dyn_cast(Op1); - auto* Op2C = dyn_cast(Op2); - - // and X, 0 - if (Opcode == ISD::AND && !Op1C && Op2C && Op2C->isNullValue()) { - return false; - } - - // or (and X, 0), Y - if (Opcode == ISD::OR) { - if (Op1->getOpcode() == ISD::AND) { - auto* Op11 = Op1->getOperand(0).getNode(); - auto* Op12 = Op1->getOperand(1).getNode(); - auto* Op11C = dyn_cast(Op11); - auto* Op12C = dyn_cast(Op12); - if (!Op11C && Op12C && Op12C->isNullValue()) { - return false; - } - } - if (Op1->getOpcode() == ISD::TRUNCATE) { - // or (trunc (and %0, 0)), Y - auto* Op11 = Op1->getOperand(0).getNode(); - if (Op11->getOpcode() == ISD::AND) { - auto* Op111 = Op11->getOperand(0).getNode(); - auto* Op112 = Op11->getOperand(1).getNode(); - auto* Op111C = dyn_cast(Op111); - auto* Op112C = dyn_cast(Op112); - if (!Op111C && Op112C && Op112C->isNullValue()) { - // or (and X, 0), Y - return false; - } - } - } - } - } - - // trunc (and X, 0) - if (Opcode == ISD::TRUNCATE) { - auto* Op1 = Op.getOperand(0).getNode(); - if (Op1->getOpcode() == ISD::AND) { - auto* Op11 = Op1->getOperand(0).getNode(); - auto* Op12 = Op1->getOperand(1).getNode(); - auto* Op11C = dyn_cast(Op11); - auto* Op12C = dyn_cast(Op12); - if (!Op11C && Op12C && Op12C->isNullValue()) { - return false; - } - } - } - if (!TLI.SimplifyDemandedBits(Op, Demanded, KnownZero, KnownOne, TLO)) return false; @@ -3098,22 +3042,6 @@ SDValue DAGCombiner::visitAND(SDNode *N) { // fold (and c1, c2) -> c1&c2 ConstantSDNode *N0C = getAsNonOpaqueConstant(N0); 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 && 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); // canonicalize constant to RHS diff --git a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp index e845a9919ef..893871f9448 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp @@ -3465,14 +3465,10 @@ SDValue SelectionDAG::getNode(unsigned Opcode, SDLoc DL, EVT VT, SDValue N1, assert(VT.isInteger() && "This operator does not apply to FP types!"); assert(N1.getValueType() == N2.getValueType() && N1.getValueType() == VT && "Binary operator types must match!"); - - // XXX-disabled: - /* // (X & 0) -> 0. This commonly occurs when legalizing i64 values, so it's // worth handling here. if (N2C && N2C->isNullValue()) return N2; - */ if (N2C && N2C->isAllOnesValue()) // X & -1 -> X return N1; break; diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp index 491d7ec451c..c075da4738a 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp @@ -645,50 +645,6 @@ bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) { // at this point. FuncInfo->clear(); - // XXX-update: Right after instruction selection, check through the - // intentionally added fake conditional branches and mark them as unremovable. - for (auto& MBB : *MF) { - // Check whether MBB has two successors which only contains an unconditional - // branch to the same destination. - if (MBB.succ_size() != 2 || - !MBB.getLastNonDebugInstr()->isUnconditionalBranch()) { - continue; - } - auto MBBSuccIter = MBB.succ_begin(); - auto* Succ1 = *MBBSuccIter; - MBBSuccIter++; - auto* Succ2 = *MBBSuccIter; - - MachineBasicBlock* Succ1Succ = nullptr; - MachineBasicBlock* Succ2Succ = nullptr; - if ((Succ1->size() == 1 && Succ1->begin()->isUnconditionalBranch()) || - (Succ1->size() == 0)) { - if (Succ1->succ_size()) { - Succ1Succ = *Succ1->succ_begin(); - } - } - if ((Succ2->size() == 1 && Succ2->begin()->isUnconditionalBranch()) || - (Succ2->size() == 0)) { - if (Succ1->succ_size()) { - Succ2Succ = *Succ2->succ_begin(); - } - } - - bool HasCommonDest = Succ1Succ && Succ1Succ == Succ2Succ; - if (HasCommonDest) { - auto MBBIter = MBB.end(); - std::advance(MBBIter, -2); - assert(MBBIter->isConditionalBranch()); - MBB.disableCanEliminateMachineBB(); - Succ1->disableCanEliminateMachineBB(); - Succ2->disableCanEliminateMachineBB(); - Succ1Succ->disableCanEliminateMachineBB(); - DEBUG(dbgs() << "Mark as unremovable machine basic block: " << MBB - << "\nMark as unremovable branch instruction: " << *MBBIter - << "\n"); - } - } - DEBUG(dbgs() << "*** MachineFunction at end of ISel ***\n"); DEBUG(MF->print(dbgs())); diff --git a/lib/IR/BasicBlock.cpp b/lib/IR/BasicBlock.cpp index 8e3cac27f48..f61276fd436 100644 --- a/lib/IR/BasicBlock.cpp +++ b/lib/IR/BasicBlock.cpp @@ -40,8 +40,7 @@ template class llvm::SymbolTableListTraits; BasicBlock::BasicBlock(LLVMContext &C, const Twine &Name, Function *NewParent, BasicBlock *InsertBefore) - : Value(Type::getLabelTy(C), Value::BasicBlockVal), Parent(nullptr), - canEliminateBlock(true) { + : Value(Type::getLabelTy(C), Value::BasicBlockVal), Parent(nullptr) { if (NewParent) insertInto(NewParent, InsertBefore); diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp index 37b56f8efc7..9198b0e1fb5 100644 --- a/lib/IR/Verifier.cpp +++ b/lib/IR/Verifier.cpp @@ -1974,10 +1974,6 @@ void Verifier::visitBasicBlock(BasicBlock &BB) { "PHI nodes must have at least one entry. If the block is dead, " "the PHI should be removed!", PN); - if (PN->getNumIncomingValues() != Preds.size()) { - dbgs() << "Problematic function: \n" << *PN->getParent()->getParent() << "\n"; - dbgs() << "Problematic block: \n" << *PN->getParent() << "\n"; - } Assert(PN->getNumIncomingValues() == Preds.size(), "PHINode should have one entry for each predecessor of its " "parent basic block!", @@ -3358,9 +3354,6 @@ void Verifier::verifyDominatesUse(Instruction &I, unsigned i) { } const Use &U = I.getOperandUse(i); - if (!(InstsInThisBlock.count(Op) || DT.dominates(Op, U))) { - dbgs() << "Problematic function: \n" << *I.getParent()->getParent() << "\n"; - } Assert(InstsInThisBlock.count(Op) || DT.dominates(Op, U), "Instruction does not dominate all uses!", Op, &I); } diff --git a/lib/Target/AArch64/AArch64TargetMachine.cpp b/lib/Target/AArch64/AArch64TargetMachine.cpp index 8cc3ff22725..c52c5544fc7 100644 --- a/lib/Target/AArch64/AArch64TargetMachine.cpp +++ b/lib/Target/AArch64/AArch64TargetMachine.cpp @@ -216,9 +216,6 @@ void AArch64PassConfig::addIRPasses() { // Always expand atomic operations, we don't deal with atomicrmw or cmpxchg // ourselves. addPass(createAtomicExpandPass(TM)); - // XXX-update: Immediate add -licm pass after atomic expand pass to deal with - // loop invariants introduced mannually. -// addPass(createLICMPass()); // Cmpxchg instructions are often used with a subsequent comparison to // determine whether it succeeded. We can exploit existing control-flow in -- 2.34.1