[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
nicholas at mxc.ca
Wed Oct 12 23:20:13 PDT 2011
Bob Wilson wrote:
> Do you have any of Evan's subsequent MachineLICM changes (141744 or
> 141747)? He reverted them in 141813 today because there were some
> regressions.
Yes, I would've had those. I'll retest again tomorrow. Thanks!
Nick
>
> 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
>> <mailto: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
>> <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
>> <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
>> <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
>> <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
>> <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
>> <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 <mailto: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 <mailto: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
More information about the llvm-commits
mailing list