[llvm-commits] [llvm] r131176 - in /llvm/trunk: lib/CodeGen/BranchFolding.cpp lib/CodeGen/BranchFolding.h lib/CodeGen/IfConversion.cpp test/CodeGen/X86/hoist-common.ll
Evan Cheng
evan.cheng at apple.com
Wed May 11 11:37:51 PDT 2011
Rafael, can you provide more information? Which buildbot? What's the symptom? I noticed http://google1.osuosl.org:8011/builders/clang-x86_64-linux-fnt is still red.
Evan
On May 10, 2011, at 8:27 PM, Rafael Espindola wrote:
> Author: rafael
> Date: Tue May 10 22:27:17 2011
> New Revision: 131176
>
> URL: http://llvm.org/viewvc/llvm-project?rev=131176&view=rev
> Log:
> Revert 131172 as it is causing clang to miscompile itself. I will try
> to provide a reduced testcase.
>
> Removed:
> llvm/trunk/test/CodeGen/X86/hoist-common.ll
> Modified:
> llvm/trunk/lib/CodeGen/BranchFolding.cpp
> llvm/trunk/lib/CodeGen/BranchFolding.h
> llvm/trunk/lib/CodeGen/IfConversion.cpp
>
> Modified: llvm/trunk/lib/CodeGen/BranchFolding.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/BranchFolding.cpp?rev=131176&r1=131175&r2=131176&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/BranchFolding.cpp (original)
> +++ llvm/trunk/lib/CodeGen/BranchFolding.cpp Tue May 10 22:27:17 2011
> @@ -41,7 +41,6 @@
> STATISTIC(NumDeadBlocks, "Number of dead blocks removed");
> STATISTIC(NumBranchOpts, "Number of branches optimized");
> STATISTIC(NumTailMerge , "Number of block tails merged");
> -STATISTIC(NumHoist , "Number of times common instructions are hoisted");
>
> static cl::opt<cl::boolOrDefault> FlagEnableTailMerge("enable-tail-merge",
> cl::init(cl::BOU_UNSET), cl::Hidden);
> @@ -66,7 +65,7 @@
> public:
> static char ID;
> explicit BranchFolderPass(bool defaultEnableTailMerge)
> - : MachineFunctionPass(ID), BranchFolder(defaultEnableTailMerge, true) {}
> + : MachineFunctionPass(ID), BranchFolder(defaultEnableTailMerge) {}
>
> virtual bool runOnMachineFunction(MachineFunction &MF);
> virtual const char *getPassName() const { return "Control Flow Optimizer"; }
> @@ -87,14 +86,12 @@
> }
>
>
> -BranchFolder::BranchFolder(bool defaultEnableTailMerge, bool CommonHoist) {
> +BranchFolder::BranchFolder(bool defaultEnableTailMerge) {
> switch (FlagEnableTailMerge) {
> case cl::BOU_UNSET: EnableTailMerge = defaultEnableTailMerge; break;
> case cl::BOU_TRUE: EnableTailMerge = true; break;
> case cl::BOU_FALSE: EnableTailMerge = false; break;
> }
> -
> - EnableHoistCommonCode = CommonHoist;
> }
>
> /// RemoveDeadBlock - Remove the specified dead machine basic block from the
> @@ -189,10 +186,9 @@
>
> bool MadeChangeThisIteration = true;
> while (MadeChangeThisIteration) {
> - MadeChangeThisIteration = TailMergeBlocks(MF);
> - MadeChangeThisIteration |= OptimizeBranches(MF);
> - if (EnableHoistCommonCode)
> - MadeChangeThisIteration |= HoistCommonCode(MF);
> + MadeChangeThisIteration = false;
> + MadeChangeThisIteration |= TailMergeBlocks(MF);
> + MadeChangeThisIteration |= OptimizeBranches(MF);
> MadeChange |= MadeChangeThisIteration;
> }
>
> @@ -914,8 +910,7 @@
> // Make sure blocks are numbered in order
> MF.RenumberBlocks();
>
> - for (MachineFunction::iterator I = llvm::next(MF.begin()), E = MF.end();
> - I != E; ) {
> + for (MachineFunction::iterator I = ++MF.begin(), E = MF.end(); I != E; ) {
> MachineBasicBlock *MBB = I++;
> MadeChange |= OptimizeBlock(MBB);
>
> @@ -1344,253 +1339,3 @@
>
> return MadeChange;
> }
> -
> -//===----------------------------------------------------------------------===//
> -// Hoist Common Code
> -//===----------------------------------------------------------------------===//
> -
> -/// HoistCommonCode - Hoist common instruction sequences at the start of basic
> -/// blocks to their common predecessor.
> -/// NOTE: This optimization does not update live-in information so it must be
> -/// run after all passes that require correct liveness information.
> -bool BranchFolder::HoistCommonCode(MachineFunction &MF) {
> - bool MadeChange = false;
> - for (MachineFunction::iterator I = MF.begin(), E = MF.end(); I != E; ) {
> - MachineBasicBlock *MBB = I++;
> - MadeChange |= HoistCommonCodeInSuccs(MBB);
> - }
> -
> - return MadeChange;
> -}
> -
> -/// findFalseBlock - BB has a fallthrough. Find its 'false' successor given
> -/// its 'true' successor.
> -static MachineBasicBlock *findFalseBlock(MachineBasicBlock *BB,
> - MachineBasicBlock *TrueBB) {
> - for (MachineBasicBlock::succ_iterator SI = BB->succ_begin(),
> - E = BB->succ_end(); SI != E; ++SI) {
> - MachineBasicBlock *SuccBB = *SI;
> - if (SuccBB != TrueBB)
> - return SuccBB;
> - }
> - return NULL;
> -}
> -
> -/// findHoistingInsertPosAndDeps - Find the location to move common instructions
> -/// in successors to. The location is ususally just before the terminator,
> -/// however if the terminator is a conditional branch and its previous
> -/// instruction is the flag setting instruction, the previous instruction is
> -/// the preferred location. This function also gathers uses and defs of the
> -/// instructions from the insertion point to the end of the block. The data is
> -/// used by HoistCommonCodeInSuccs to ensure safety.
> -static
> -MachineBasicBlock::iterator findHoistingInsertPosAndDeps(MachineBasicBlock *MBB,
> - const TargetInstrInfo *TII,
> - const TargetRegisterInfo *TRI,
> - SmallSet<unsigned,4> &Uses,
> - SmallSet<unsigned,4> &Defs) {
> - MachineBasicBlock::iterator Loc = MBB->getFirstTerminator();
> - if (!TII->isUnpredicatedTerminator(Loc))
> - return MBB->end();
> -
> - for (unsigned i = 0, e = Loc->getNumOperands(); i != e; ++i) {
> - const MachineOperand &MO = Loc->getOperand(i);
> - if (!MO.isReg())
> - continue;
> - unsigned Reg = MO.getReg();
> - if (!Reg)
> - continue;
> - if (MO.isUse()) {
> - Uses.insert(Reg);
> - for (const unsigned *AS = TRI->getAliasSet(Reg); *AS; ++AS)
> - Uses.insert(*AS);
> - } else if (!MO.isDead())
> - // Don't try to hoist code in the rare case the terminator defines a
> - // register that is later used.
> - return MBB->end();
> - }
> -
> - if (Uses.empty())
> - return Loc;
> - if (Loc == MBB->begin())
> - return MBB->end();
> -
> - // The terminator is probably a conditional branch, try not to separate the
> - // branch from condition setting instruction.
> - MachineBasicBlock::iterator PI = Loc;
> - --PI;
> - while (PI != MBB->begin() && Loc->isDebugValue())
> - --PI;
> -
> - bool IsDef = false;
> - for (unsigned i = 0, e = PI->getNumOperands(); !IsDef && i != e; ++i) {
> - const MachineOperand &MO = PI->getOperand(i);
> - if (!MO.isReg() || MO.isUse())
> - continue;
> - unsigned Reg = MO.getReg();
> - if (!Reg)
> - continue;
> - if (Uses.count(Reg))
> - IsDef = true;
> - }
> - if (!IsDef)
> - // The condition setting instruction is not just before the conditional
> - // branch.
> - return Loc;
> -
> - // Be conservative, don't insert instruction above something that may have
> - // side-effects. And since it's potentially bad to separate flag setting
> - // instruction from the conditional branch, just abort the optimization
> - // completely.
> - // Also avoid moving code above predicated instruction since it's hard to
> - // reason about register liveness with predicated instruction.
> - bool DontMoveAcrossStore = true;
> - if (!PI->isSafeToMove(TII, 0, DontMoveAcrossStore) ||
> - TII->isPredicated(PI))
> - return MBB->end();
> -
> -
> - // Find out what registers are live. Note this routine is ignoring other live
> - // registers which are only used by instructions in successor blocks.
> - for (unsigned i = 0, e = PI->getNumOperands(); i != e; ++i) {
> - const MachineOperand &MO = PI->getOperand(i);
> - if (!MO.isReg())
> - continue;
> - unsigned Reg = MO.getReg();
> - if (!Reg)
> - continue;
> - if (MO.isUse()) {
> - Uses.insert(Reg);
> - for (const unsigned *AS = TRI->getAliasSet(Reg); *AS; ++AS)
> - Uses.insert(*AS);
> - } else {
> - if (Uses.count(Reg)) {
> - Uses.erase(Reg);
> - for (const unsigned *SR = TRI->getSubRegisters(Reg); *SR; ++SR)
> - Uses.erase(*SR); // Use getSubRegisters to be conservative
> - Defs.insert(Reg);
> - for (const unsigned *AS = TRI->getAliasSet(Reg); *AS; ++AS)
> - Defs.insert(*AS);
> - }
> - }
> - }
> -
> - return PI;
> -}
> -
> -/// HoistCommonCodeInSuccs - If the successors of MBB has common instruction
> -/// sequence at the start of the function, move the instructions before MBB
> -/// terminator if it's legal.
> -bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) {
> - MachineBasicBlock *TBB = 0, *FBB = 0;
> - SmallVector<MachineOperand, 4> Cond;
> - if (TII->AnalyzeBranch(*MBB, TBB, FBB, Cond, true) || !TBB || Cond.empty())
> - return false;
> -
> - if (!FBB) FBB = findFalseBlock(MBB, TBB);
> - if (!FBB)
> - // Malformed bcc? True and false blocks are the same?
> - return false;
> -
> - // Restrict the optimization to cases where MBB is the only predecessor,
> - // it is an obvious win.
> - if (TBB->pred_size() > 1 || FBB->pred_size() > 1)
> - return false;
> -
> - // Find a suitable position to hoist the common instructions to. Also figure
> - // out which registers are used or defined by instructions from the insertion
> - // point to the end of the block.
> - SmallSet<unsigned, 4> Uses, Defs;
> - MachineBasicBlock::iterator Loc =
> - findHoistingInsertPosAndDeps(MBB, TII, TRI, Uses, Defs);
> - if (Loc == MBB->end())
> - return false;
> -
> - SmallSet<unsigned, 4> LocalDefs;
> - unsigned NumDups = 0;
> - MachineBasicBlock::iterator TIB = TBB->begin();
> - MachineBasicBlock::iterator FIB = FBB->begin();
> - MachineBasicBlock::iterator TIE = TBB->end();
> - MachineBasicBlock::iterator FIE = FBB->end();
> - while (TIB != TIE && FIB != FIE) {
> - // Skip dbg_value instructions. These do not count.
> - if (TIB->isDebugValue()) {
> - while (TIB != TIE && TIB->isDebugValue())
> - ++TIB;
> - if (TIB == TIE)
> - break;
> - }
> - if (FIB->isDebugValue()) {
> - while (FIB != FIE && FIB->isDebugValue())
> - ++FIB;
> - if (FIB == FIE)
> - break;
> - }
> - if (!TIB->isIdenticalTo(FIB))
> - break;
> -
> - if (TII->isPredicated(TIB))
> - // Hard to reason about register liveness with predicated instruction.
> - break;
> -
> - bool IsSafe = true;
> - for (unsigned i = 0, e = TIB->getNumOperands(); i != e; ++i) {
> - const MachineOperand &MO = TIB->getOperand(i);
> - if (!MO.isReg())
> - continue;
> - unsigned Reg = MO.getReg();
> - if (!Reg)
> - continue;
> - if (MO.isDef()) {
> - if (Uses.count(Reg)) {
> - // Avoid clobbering a register that's used by the instruction at
> - // the point of insertion.
> - IsSafe = false;
> - break;
> - }
> - if (!MO.isDead() && Defs.count(Reg)) {
> - // Don't hoist the instruction if the def would be clobber by the
> - // instruction at the point insertion. FIXME: This is overly
> - // conservative. It should be possible to hoist the instructions
> - // in BB2 in the following example:
> - // BB1:
> - // r1, eflag = op1 r2, r3
> - // brcc eflag
> - //
> - // BB2:
> - // r1 = op2, ...
> - // = op3, r1<kill>
> - IsSafe = false;
> - break;
> - }
> - LocalDefs.insert(Reg);
> - for (const unsigned *SR = TRI->getSubRegisters(Reg); *SR; ++SR)
> - LocalDefs.insert(*SR);
> - } else if (!LocalDefs.count(Reg)) {
> - if (Defs.count(Reg)) {
> - // Use is defined by the instruction at the point of insertion.
> - IsSafe = false;
> - break;
> - }
> - }
> - }
> - if (!IsSafe)
> - break;
> -
> - bool DontMoveAcrossStore = true;
> - if (!TIB->isSafeToMove(TII, 0, DontMoveAcrossStore))
> - break;
> -
> - ++NumDups;
> - ++TIB;
> - ++FIB;
> - }
> -
> - if (!NumDups)
> - return false;
> -
> - MBB->splice(Loc, TBB, TBB->begin(), TIB);
> - FBB->erase(FBB->begin(), FIB);
> - ++NumHoist;
> - return true;
> -}
>
> Modified: llvm/trunk/lib/CodeGen/BranchFolding.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/BranchFolding.h?rev=131176&r1=131175&r2=131176&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/BranchFolding.h (original)
> +++ llvm/trunk/lib/CodeGen/BranchFolding.h Tue May 10 22:27:17 2011
> @@ -19,10 +19,11 @@
> class RegScavenger;
> class TargetInstrInfo;
> class TargetRegisterInfo;
> + template<typename T> class SmallVectorImpl;
>
> class BranchFolder {
> public:
> - explicit BranchFolder(bool defaultEnableTailMerge, bool CommonHoist);
> + explicit BranchFolder(bool defaultEnableTailMerge);
>
> bool OptimizeFunction(MachineFunction &MF,
> const TargetInstrInfo *tii,
> @@ -84,7 +85,6 @@
> std::vector<SameTailElt> SameTails;
>
> bool EnableTailMerge;
> - bool EnableHoistCommonCode;
> const TargetInstrInfo *TII;
> const TargetRegisterInfo *TRI;
> MachineModuleInfo *MMI;
> @@ -110,9 +110,6 @@
> bool OptimizeBlock(MachineBasicBlock *MBB);
> void RemoveDeadBlock(MachineBasicBlock *MBB);
> bool OptimizeImpDefsBlock(MachineBasicBlock *MBB);
> -
> - bool HoistCommonCode(MachineFunction &MF);
> - bool HoistCommonCodeInSuccs(MachineBasicBlock *MBB);
> };
> }
>
>
> Modified: llvm/trunk/lib/CodeGen/IfConversion.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/IfConversion.cpp?rev=131176&r1=131175&r2=131176&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/IfConversion.cpp (original)
> +++ llvm/trunk/lib/CodeGen/IfConversion.cpp Tue May 10 22:27:17 2011
> @@ -265,7 +265,7 @@
> if (!TII) return false;
>
> // Tail merge tend to expose more if-conversion opportunities.
> - BranchFolder BF(true, false);
> + BranchFolder BF(true);
> bool BFChange = BF.OptimizeFunction(MF, TII,
> MF.getTarget().getRegisterInfo(),
> getAnalysisIfAvailable<MachineModuleInfo>());
> @@ -399,7 +399,7 @@
> BBAnalysis.clear();
>
> if (MadeChange && IfCvtBranchFold) {
> - BranchFolder BF(false, false);
> + BranchFolder BF(false);
> BF.OptimizeFunction(MF, TII,
> MF.getTarget().getRegisterInfo(),
> getAnalysisIfAvailable<MachineModuleInfo>());
>
> Removed: llvm/trunk/test/CodeGen/X86/hoist-common.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/hoist-common.ll?rev=131175&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/hoist-common.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/hoist-common.ll (removed)
> @@ -1,28 +0,0 @@
> -; RUN: llc < %s -march=x86-64 | FileCheck %s
> -
> -; Common "xorb al, al" instruction in the two successor blocks should be
> -; moved to the entry block above the test + je.
> -
> -; rdar://9145558
> -
> -define zeroext i1 @t(i32 %c) nounwind ssp {
> -entry:
> -; CHECK: t:
> -; CHECK: xorb %al, %al
> -; CHECK: test
> -; CHECK: je
> - %tobool = icmp eq i32 %c, 0
> - br i1 %tobool, label %return, label %if.then
> -
> -if.then:
> -; CHECK: callq
> - %call = tail call zeroext i1 (...)* @foo() nounwind
> - br label %return
> -
> -return:
> -; CHECK: ret
> - %retval.0 = phi i1 [ %call, %if.then ], [ false, %entry ]
> - ret i1 %retval.0
> -}
> -
> -declare zeroext i1 @foo(...)
>
>
> _______________________________________________
> 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