[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
Bob Wilson
bob.wilson at apple.com
Wed May 11 21:56:48 PDT 2011
I suspect that's also related to PR9858. Targetdata strings starting with "-", i.e., with no endianness specified, suddenly started causing assertion failures, and that buildbot is running on osu7.
Nick, can you take a look at that PR?
On May 11, 2011, at 2:27 PM, Nick Lewycky wrote:
> I recently upgraded that machine to gcc 4.6. We may need to fix the testsuite to build with newer GCCs.
>
> Nick
>
> On 11 May 2011 13:39, Jim Grosbach <grosbach at apple.com> wrote:
> There's something odd going on with that buildbot. There are compile-time errors from the system host compiler on tramp3d, for example, as well as errors from the clang under test.
>
> On May 11, 2011, at 11:37 AM, Evan Cheng wrote:
>
> > 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
> >
> > _______________________________________________
> > 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
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110511/2cabba2e/attachment.html>
More information about the llvm-commits
mailing list