[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
Nick Lewycky
nlewycky at google.com
Wed Oct 12 19:24:18 PDT 2011
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20111012/1724877b/attachment.html>
More information about the llvm-commits
mailing list