[llvm-commits] [llvm] r141689 - in /llvm/trunk: lib/CodeGen/MachineLICM.cpp test/CodeGen/ARM/lsr-unfolded-offset.ll test/CodeGen/X86/licm-dominance.ll test/CodeGen/X86/licm-nested.ll test/CodeGen/X86/sink-hoist.ll
Bob Wilson
bob.wilson at apple.com
Wed Oct 12 23:11:44 PDT 2011
Do you have any of Evan's subsequent MachineLICM changes (141744 or 141747)? He reverted them in 141813 today because there were some regressions.
On Oct 12, 2011, at 7:24 PM, Nick Lewycky wrote:
> Hey Devang! licm-dominance.ll is causing me some trouble. On my machine with the usual make-based build, I see this:
>
> nlewycky at ducttape:~/llvm$ Debug+Asserts/bin/llc -asm-verbose=false test/CodeGen/X86/licm-dominance.ll -o -
> .section __TEXT,__text,regular,pure_instructions
> .globl _CMSColorWorldCreateParametricData
> _CMSColorWorldCreateParametricData:
> Ltmp0:
> .cfi_startproc
> xorb %al, %al
> LBB0_1:
> testb %al, %al
> jne LBB0_3
> xorb %al, %al
> testb %al, %al
> LBB0_3:
> testb %al, %al
> jne LBB0_1
> ret
> Ltmp1:
> .cfi_endproc
> Leh_func_end0:
>
>
> .subsections_via_symbols
>
> While on a Google-style build (similar but passes things like -funsigned-char), I get this:
>
> .section __TEXT,__text,regular,pure_instructions
> .globl _CMSColorWorldCreateParametricData
> _CMSColorWorldCreateParametricData:
> Ltmp0:
> .cfi_startproc
> xorb %al, %al
> LBB0_1:
> testb %al, %al
> jne LBB0_3
> testb %al, %al
> LBB0_3:
> testb %al, %al
> jne LBB0_1
> Ltmp1:
> .cfi_endproc
> Leh_func_end0:
>
>
> .subsections_via_symbols
>
> Note that two instructions were removed. A build with clang -O1 produces this valgrind error on the same testcase:
>
> ==12757== Conditional jump or move depends on uninitialised value(s)
> ==12757== at 0x80BAB6: (anonymous namespace)::PeepholeOptimizer::runOnMachineFunction(llvm::MachineFunction&) (third_party/llvm/llvm/lib/CodeGen/PeepholeOptimizer.cpp:138)
> ==12757== by 0x80D482: llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (third_party/llvm/llvm/lib/CodeGen/MachineFunctionPass.cpp:33)
> ==12757== by 0x9C0549: llvm::FPPassManager::runOnFunction(llvm::Function&) (third_party/llvm/llvm/lib/VMCore/PassManager.cpp:1512)
> ==12757== by 0x9C07FA: llvm::FPPassManager::runOnModule(llvm::Module&) (third_party/llvm/llvm/lib/VMCore/PassManager.cpp:1534)
> ==12757== by 0x9C099D: llvm::MPPassManager::runOnModule(llvm::Module&) (third_party/llvm/llvm/lib/VMCore/PassManager.cpp:1588)
> ==12757== by 0x9C0E74: llvm::PassManagerImpl::run(llvm::Module&) (third_party/llvm/llvm/lib/VMCore/PassManager.cpp:1672)
> ==12757== by 0x9C133C: llvm::PassManager::run(llvm::Module&) (third_party/llvm/llvm/lib/VMCore/PassManager.cpp:1716)
> ==12757== by 0x4E6EBF: main (third_party/llvm/llvm/tools/llc/llc.cpp:374)
>
> but doing anything (ie., errs() << "this: " << this;) will cause the error to go away. I can't find anything wrong with your commit, or the code that valgrind is pointing to, but I'm hoping that you or someone on the list will be able to help track it down...
>
> Nick
>
> On 11 October 2011 11:09, Devang Patel <dpatel at apple.com> wrote:
> Author: dpatel
> Date: Tue Oct 11 13:09:58 2011
> New Revision: 141689
>
> URL: http://llvm.org/viewvc/llvm-project?rev=141689&view=rev
> Log:
> Add dominance check for the instruction being hoisted.
>
> For example, MachineLICM should not hoist a load that is not guaranteed to be executed.
> Radar 10254254.
>
>
> Added:
> llvm/trunk/test/CodeGen/X86/licm-dominance.ll
> Modified:
> llvm/trunk/lib/CodeGen/MachineLICM.cpp
> llvm/trunk/test/CodeGen/ARM/lsr-unfolded-offset.ll
> llvm/trunk/test/CodeGen/X86/licm-nested.ll
> llvm/trunk/test/CodeGen/X86/sink-hoist.ll
>
> Modified: llvm/trunk/lib/CodeGen/MachineLICM.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineLICM.cpp?rev=141689&r1=141688&r2=141689&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MachineLICM.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineLICM.cpp Tue Oct 11 13:09:58 2011
> @@ -91,6 +91,11 @@
> // For each opcode, keep a list of potential CSE instructions.
> DenseMap<unsigned, std::vector<const MachineInstr*> > CSEMap;
>
> + // If a MBB does not dominate loop exiting blocks then it may not safe
> + // to hoist loads from this block.
> + bool CurrentMBBDominatesLoopExitingBlocks;
> + bool NeedToCheckMBBDominance;
> +
> public:
> static char ID; // Pass identification, replacement for typeid
> MachineLICM() :
> @@ -194,6 +199,10 @@
> /// hoist the given loop invariant.
> bool IsProfitableToHoist(MachineInstr &MI);
>
> + /// IsGuaranteedToExecute - Check if this mbb is guaranteed to execute.
> + /// If not then a load from this mbb may not be safe to hoist.
> + bool IsGuaranteedToExecute(MachineBasicBlock *BB);
> +
> /// HoistRegion - Walk the specified region of the CFG (defined by all
> /// blocks dominated by the specified block, and that are in the current
> /// loop) in depth first order w.r.t the DominatorTree. This allows us to
> @@ -295,6 +304,9 @@
> MRI = &MF.getRegInfo();
> InstrItins = TM->getInstrItineraryData();
> AllocatableSet = TRI->getAllocatableSet(MF);
> + // Stay conservative.
> + CurrentMBBDominatesLoopExitingBlocks = false;
> + NeedToCheckMBBDominance = true;
>
> if (PreRegAlloc) {
> // Estimate register pressure during pre-regalloc pass.
> @@ -459,6 +471,7 @@
> ++PhysRegDefs[*AS];
> }
>
> + NeedToCheckMBBDominance = true;
> for (MachineBasicBlock::iterator
> MII = BB->begin(), E = BB->end(); MII != E; ++MII) {
> MachineInstr *MI = &*MII;
> @@ -552,6 +565,30 @@
> Changed = true;
> }
>
> +// IsGuaranteedToExecute - Check if this mbb is guaranteed to execute.
> +// If not then a load from this mbb may not be safe to hoist.
> +bool MachineLICM::IsGuaranteedToExecute(MachineBasicBlock *BB) {
> + // Do not check if we already have checked it once.
> + if (NeedToCheckMBBDominance == false)
> + return CurrentMBBDominatesLoopExitingBlocks;
> +
> + NeedToCheckMBBDominance = false;
> +
> + if (BB != CurLoop->getHeader()) {
> + // Check loop exiting blocks.
> + SmallVector<MachineBasicBlock*, 8> CurrentLoopExitingBlocks;
> + CurLoop->getExitingBlocks(CurrentLoopExitingBlocks);
> + for (unsigned i = 0, e = CurrentLoopExitingBlocks.size(); i != e; ++i)
> + if (!DT->dominates(BB, CurrentLoopExitingBlocks[i])) {
> + CurrentMBBDominatesLoopExitingBlocks = false;
> + return CurrentMBBDominatesLoopExitingBlocks;
> + }
> + }
> +
> + CurrentMBBDominatesLoopExitingBlocks = true;
> + return CurrentMBBDominatesLoopExitingBlocks;
> +}
> +
> /// HoistRegion - Walk the specified region of the CFG (defined by all blocks
> /// dominated by the specified block, and that are in the current loop) in depth
> /// first order w.r.t the DominatorTree. This allows us to visit definitions
> @@ -578,6 +615,7 @@
> // Remember livein register pressure.
> BackTrace.push_back(RegPressure);
>
> + NeedToCheckMBBDominance = true;
> for (MachineBasicBlock::iterator
> MII = BB->begin(), E = BB->end(); MII != E; ) {
> MachineBasicBlock::iterator NextMII = MII; ++NextMII;
> @@ -711,7 +749,14 @@
> bool DontMoveAcrossStore = true;
> if (!I.isSafeToMove(TII, AA, DontMoveAcrossStore))
> return false;
> -
> +
> + // If it is load then check if it is guaranteed to execute by making sure that
> + // it dominates all exiting blocks. If it doesn't, then there is a path out of
> + // the loop which does not execute this load, so we can't hoist it.
> + // Stores and side effects are already checked by isSafeToMove.
> + if (I.getDesc().mayLoad() && !IsGuaranteedToExecute(I.getParent()))
> + return false;
> +
> return true;
> }
>
>
> Modified: llvm/trunk/test/CodeGen/ARM/lsr-unfolded-offset.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/lsr-unfolded-offset.ll?rev=141689&r1=141688&r2=141689&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/ARM/lsr-unfolded-offset.ll (original)
> +++ llvm/trunk/test/CodeGen/ARM/lsr-unfolded-offset.ll Tue Oct 11 13:09:58 2011
> @@ -4,12 +4,11 @@
> ; register pressure and therefore spilling. There is more room for improvement
> ; here.
>
> -; CHECK: sub sp, #{{32|28|24}}
> +; CHECK: sub sp, #{{40|32|28|24}}
>
> ; CHECK: %for.inc
> ; CHECK: ldr{{(.w)?}} r{{.*}}, [sp, #
> ; CHECK: ldr{{(.w)?}} r{{.*}}, [sp, #
> -; CHECK: ldr{{(.w)?}} r{{.*}}, [sp, #
> ; CHECK: add
>
> target datalayout = "e-p:32:32:32-i1:8:32-i8:8:32-i16:16:32-i32:32:32-i64:32:32-f32:32:32-f64:32:32-v64:32:64-v128:32:128-a0:0:32-n32"
>
> Added: llvm/trunk/test/CodeGen/X86/licm-dominance.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/licm-dominance.ll?rev=141689&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/licm-dominance.ll (added)
> +++ llvm/trunk/test/CodeGen/X86/licm-dominance.ll Tue Oct 11 13:09:58 2011
> @@ -0,0 +1,36 @@
> +; RUN: llc -asm-verbose=false < %s | FileCheck %s
> +
> +; MachineLICM should check dominance before hoisting instructions.
> +; CHECK: jne LBB0_3
> +; CHECK-NEXT: xorb %al, %al
> +; CHECK-NEXT: testb %al, %al
> +
> +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
> +target triple = "x86_64-apple-macosx10.7.2"
> +
> +define void @CMSColorWorldCreateParametricData() nounwind uwtable optsize ssp {
> +entry:
> + br label %for.body.i
> +
> +for.body.i:
> + br i1 undef, label %for.inc.i, label %if.then26.i
> +
> +if.then26.i:
> + br i1 undef, label %if.else.i.i, label %lor.lhs.false.i.i
> +
> +if.else.i.i:
> + br i1 undef, label %lor.lhs.false.i.i, label %if.then116.i.i
> +
> +lor.lhs.false.i.i:
> + br i1 undef, label %for.inc.i, label %if.then116.i.i
> +
> +if.then116.i.i:
> + unreachable
> +
> +for.inc.i:
> + %cmp17.i = icmp ult i64 undef, undef
> + br i1 %cmp17.i, label %for.body.i, label %if.end28.i
> +
> +if.end28.i:
> + ret void
> +}
>
> Modified: llvm/trunk/test/CodeGen/X86/licm-nested.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/licm-nested.ll?rev=141689&r1=141688&r2=141689&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/licm-nested.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/licm-nested.ll Tue Oct 11 13:09:58 2011
> @@ -1,4 +1,4 @@
> -; RUN: llc -mtriple=x86_64-apple-darwin -march=x86-64 < %s -o /dev/null -stats -info-output-file - | grep machine-licm | grep 3
> +; RUN: llc -mtriple=x86_64-apple-darwin -march=x86-64 < %s -o /dev/null -stats -info-output-file - | grep machine-licm | grep 2
>
> ; MachineLICM should be able to hoist the symbolic addresses out of
> ; the inner loops.
>
> Modified: llvm/trunk/test/CodeGen/X86/sink-hoist.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/sink-hoist.ll?rev=141689&r1=141688&r2=141689&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/sink-hoist.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/sink-hoist.ll Tue Oct 11 13:09:58 2011
> @@ -102,6 +102,7 @@
> br label %bb60
>
> bb: ; preds = %bb60
> + %i.0 = phi i32 [ 0, %bb60 ] ; <i32> [#uses=2]
> %0 = bitcast float* %x_addr.0 to <4 x float>* ; <<4 x float>*> [#uses=1]
> %1 = load <4 x float>* %0, align 16 ; <<4 x float>> [#uses=4]
> %tmp20 = bitcast <4 x float> %1 to <4 x i32> ; <<4 x i32>> [#uses=1]
> @@ -129,15 +130,14 @@
> %5 = getelementptr float* %x_addr.0, i64 4 ; <float*> [#uses=1]
> %6 = getelementptr float* %y_addr.0, i64 4 ; <float*> [#uses=1]
> %7 = add i32 %i.0, 4 ; <i32> [#uses=1]
> - br label %bb60
> + %8 = load i32* %n, align 4 ; <i32> [#uses=1]
> + %9 = icmp sgt i32 %8, %7 ; <i1> [#uses=1]
> + br i1 %9, label %bb60, label %return
>
> bb60: ; preds = %bb, %entry
> - %i.0 = phi i32 [ 0, %entry ], [ %7, %bb ] ; <i32> [#uses=2]
> %x_addr.0 = phi float* [ %x, %entry ], [ %5, %bb ] ; <float*> [#uses=2]
> %y_addr.0 = phi float* [ %y, %entry ], [ %6, %bb ] ; <float*> [#uses=2]
> - %8 = load i32* %n, align 4 ; <i32> [#uses=1]
> - %9 = icmp sgt i32 %8, %i.0 ; <i1> [#uses=1]
> - br i1 %9, label %bb, label %return
> + br label %bb
>
> return: ; preds = %bb60
> ret void
>
>
> _______________________________________________
> 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/20111012/dadae8f1/attachment.html>
More information about the llvm-commits
mailing list