[llvm-commits] [llvm] r131241 - in /llvm/trunk: lib/CodeGen/BranchFolding.cpp test/CodeGen/X86/hoist-common.ll

[Galina Kistanova]

Hello,

Test llvm/trunk/test/CodeGen/X86/hoist-common.ll fails on two windows machines:

http://google1.osuosl.org:8011/builders/llvm-gcc-native-mingw32/builds/1665
http://google1.osuosl.org:8011/builders/llvm-gcc-native-mingw32-win7/builds/1921

Please see attached hoist-common.ll.out.

Thanks

Galina


On Thu, May 12, 2011 at 1:30 PM, Evan Cheng <evan.cheng at apple.com> wrote:
> Author: evancheng
> Date: Thu May 12 15:30:01 2011
> New Revision: 131241
>
> URL: http://llvm.org/viewvc/llvm-project?rev=131241&view=rev
> Log:
> Re-enable branchfolding common code hoisting optimization. Fixed a liveness test bug and also taught it to update liveins.
>
> Added:
>    llvm/trunk/test/CodeGen/X86/hoist-common.ll
> Modified:
>    llvm/trunk/lib/CodeGen/BranchFolding.cpp
>
> Modified: llvm/trunk/lib/CodeGen/BranchFolding.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/BranchFolding.cpp?rev=131241&r1=131240&r2=131241&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/BranchFolding.cpp (original)
> +++ llvm/trunk/lib/CodeGen/BranchFolding.cpp Thu May 12 15:30:01 2011
> @@ -1354,10 +1354,6 @@
>  /// 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) {
> -#if 1
> -  // FIXME: Temporarily disabled.
> -  return false;
> -#endif
>   bool MadeChange = false;
>   for (MachineFunction::iterator I = MF.begin(), E = MF.end(); I != E; ) {
>     MachineBasicBlock *MBB = I++;
> @@ -1472,10 +1468,10 @@
>         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);
>       }
> +      Defs.insert(Reg);
> +      for (const unsigned *AS = TRI->getAliasSet(Reg); *AS; ++AS)
> +        Defs.insert(*AS);
>     }
>   }
>
> @@ -1511,7 +1507,8 @@
>     return false;
>
>   bool HasDups = false;
> -  SmallSet<unsigned, 4> LocalDefs;
> +  SmallVector<unsigned, 4> LocalDefs;
> +  SmallSet<unsigned, 4> LocalDefsSet;
>   MachineBasicBlock::iterator TIB = TBB->begin();
>   MachineBasicBlock::iterator FIB = FBB->begin();
>   MachineBasicBlock::iterator TIE = TBB->end();
> @@ -1568,11 +1565,7 @@
>           IsSafe = false;
>           break;
>         }
> -
> -        LocalDefs.insert(Reg);
> -        for (const unsigned *SR = TRI->getSubRegisters(Reg); *SR; ++SR)
> -          LocalDefs.insert(*SR);
> -      } else if (!LocalDefs.count(Reg)) {
> +      } else if (!LocalDefsSet.count(Reg)) {
>         if (Defs.count(Reg)) {
>           // Use is defined by the instruction at the point of insertion.
>           IsSafe = false;
> @@ -1587,6 +1580,28 @@
>     if (!TIB->isSafeToMove(TII, 0, DontMoveAcrossStore))
>       break;
>
> +    // Track local defs so we can update liveins.
> +    for (unsigned i = 0, e = TIB->getNumOperands(); i != e; ++i) {
> +      MachineOperand &MO = TIB->getOperand(i);
> +      if (!MO.isReg())
> +        continue;
> +      unsigned Reg = MO.getReg();
> +      if (!Reg)
> +        continue;
> +      if (MO.isDef()) {
> +        if (!MO.isDead()) {
> +          LocalDefs.push_back(Reg);
> +          LocalDefsSet.insert(Reg);
> +          for (const unsigned *SR = TRI->getSubRegisters(Reg); *SR; ++SR)
> +            LocalDefsSet.insert(*SR);
> +        }
> +      } else if (MO.isKill() && LocalDefsSet.count(Reg)) {
> +        LocalDefsSet.erase(Reg);
> +        for (const unsigned *SR = TRI->getSubRegisters(Reg); *SR; ++SR)
> +          LocalDefsSet.erase(*SR);
> +      }
> +    }
> +
>     HasDups = true;;
>     ++TIB;
>     ++FIB;
> @@ -1597,6 +1612,16 @@
>
>   MBB->splice(Loc, TBB, TBB->begin(), TIB);
>   FBB->erase(FBB->begin(), FIB);
> +
> +  // Update livein's.
> +  for (unsigned i = 0, e = LocalDefs.size(); i != e; ++i) {
> +    unsigned Def = LocalDefs[i];
> +    if (LocalDefsSet.count(Def)) {
> +      TBB->addLiveIn(Def);
> +      FBB->addLiveIn(Def);
> +    }
> +  }
> +
>   ++NumHoist;
>   return true;
>  }
>
> Added: llvm/trunk/test/CodeGen/X86/hoist-common.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/hoist-common.ll?rev=131241&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/hoist-common.ll (added)
> +++ llvm/trunk/test/CodeGen/X86/hoist-common.ll Thu May 12 15:30:01 2011
> @@ -0,0 +1,28 @@
> +; 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
>
-------------- next part --------------
	.def	 t;
	.scl	2;
	.type	32;
	.endef
	.text
	.globl	t
	.align	16, 0x90
t:                                      # @t
# BB#0:                                 # %entry
	subq	$40, %rsp
	testl	%ecx, %ecx
	jne	.LBB0_2
# BB#1:
	xorb	%al, %al
	addq	$40, %rsp
	ret
.LBB0_2:                                # %if.then
	callq	foo
	addq	$40, %rsp
	ret