From 1ce75dcbbcb6a67904a23b4ec701d1e994767c7e Mon Sep 17 00:00:00 2001 From: Evan Cheng Date: Mon, 7 Jul 2008 19:51:32 +0000 Subject: [PATCH] Fix two serious LSR bugs. 1. LSR runOnLoop is always returning false regardless if any transformation is made. 2. AddUsersIfInteresting can create new instructions that are added to DeadInsts. But there is a later early exit which prevents them from being freed. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@53193 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Scalar/LoopStrengthReduce.cpp | 70 ++++++------- .../X86/2008-07-07-DanglingDeadInsts.ll | 99 +++++++++++++++++++ 2 files changed, 134 insertions(+), 35 deletions(-) create mode 100644 test/CodeGen/X86/2008-07-07-DanglingDeadInsts.ll diff --git a/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/lib/Transforms/Scalar/LoopStrengthReduce.cpp index 7a372024c9f..ccb25f05a64 100644 --- a/lib/Transforms/Scalar/LoopStrengthReduce.cpp +++ b/lib/Transforms/Scalar/LoopStrengthReduce.cpp @@ -1737,6 +1737,7 @@ void LoopStrengthReduce::OptimizeIndvars(Loop *L) { // live ranges for the IV correctly. CondUse->Offset = SE->getMinusSCEV(CondUse->Offset, *CondStride); CondUse->isUseOfPostIncrementedValue = true; + Changed = true; } bool LoopStrengthReduce::runOnLoop(Loop *L, LPPassManager &LPM) { @@ -1754,49 +1755,48 @@ bool LoopStrengthReduce::runOnLoop(Loop *L, LPPassManager &LPM) { for (BasicBlock::iterator I = L->getHeader()->begin(); isa(I); ++I) AddUsersIfInteresting(I, L, Processed); - // If we have nothing to do, return. - if (IVUsesByStride.empty()) return false; + if (!IVUsesByStride.empty()) { + // Optimize induction variables. Some indvar uses can be transformed to use + // strides that will be needed for other purposes. A common example of this + // is the exit test for the loop, which can often be rewritten to use the + // computation of some other indvar to decide when to terminate the loop. + OptimizeIndvars(L); - // Optimize induction variables. Some indvar uses can be transformed to use - // strides that will be needed for other purposes. A common example of this - // is the exit test for the loop, which can often be rewritten to use the - // computation of some other indvar to decide when to terminate the loop. - OptimizeIndvars(L); + // FIXME: We can widen subreg IV's here for RISC targets. e.g. instead of + // doing computation in byte values, promote to 32-bit values if safe. + // FIXME: Attempt to reuse values across multiple IV's. In particular, we + // could have something like "for(i) { foo(i*8); bar(i*16) }", which should + // be codegened as "for (j = 0;; j+=8) { foo(j); bar(j+j); }" on X86/PPC. + // Need to be careful that IV's are all the same type. Only works for + // intptr_t indvars. - // FIXME: We can widen subreg IV's here for RISC targets. e.g. instead of - // doing computation in byte values, promote to 32-bit values if safe. - - // FIXME: Attempt to reuse values across multiple IV's. In particular, we - // could have something like "for(i) { foo(i*8); bar(i*16) }", which should be - // codegened as "for (j = 0;; j+=8) { foo(j); bar(j+j); }" on X86/PPC. Need - // to be careful that IV's are all the same type. Only works for intptr_t - // indvars. - - // If we only have one stride, we can more aggressively eliminate some things. - bool HasOneStride = IVUsesByStride.size() == 1; + // If we only have one stride, we can more aggressively eliminate some + // things. + bool HasOneStride = IVUsesByStride.size() == 1; #ifndef NDEBUG - DOUT << "\nLSR on "; - DEBUG(L->dump()); + DOUT << "\nLSR on "; + DEBUG(L->dump()); #endif - // IVsByStride keeps IVs for one particular loop. - assert(IVsByStride.empty() && "Stale entries in IVsByStride?"); + // IVsByStride keeps IVs for one particular loop. + assert(IVsByStride.empty() && "Stale entries in IVsByStride?"); - // Sort the StrideOrder so we process larger strides first. - std::stable_sort(StrideOrder.begin(), StrideOrder.end(), StrideCompare()); + // Sort the StrideOrder so we process larger strides first. + std::stable_sort(StrideOrder.begin(), StrideOrder.end(), StrideCompare()); - // Note: this processes each stride/type pair individually. All users passed - // into StrengthReduceStridedIVUsers have the same type AND stride. Also, - // note that we iterate over IVUsesByStride indirectly by using StrideOrder. - // This extra layer of indirection makes the ordering of strides deterministic - // - not dependent on map order. - for (unsigned Stride = 0, e = StrideOrder.size(); Stride != e; ++Stride) { - std::map::iterator SI = - IVUsesByStride.find(StrideOrder[Stride]); - assert(SI != IVUsesByStride.end() && "Stride doesn't exist!"); - StrengthReduceStridedIVUsers(SI->first, SI->second, L, HasOneStride); + // Note: this processes each stride/type pair individually. All users + // passed into StrengthReduceStridedIVUsers have the same type AND stride. + // Also, note that we iterate over IVUsesByStride indirectly by using + // StrideOrder. This extra layer of indirection makes the ordering of + // strides deterministic - not dependent on map order. + for (unsigned Stride = 0, e = StrideOrder.size(); Stride != e; ++Stride) { + std::map::iterator SI = + IVUsesByStride.find(StrideOrder[Stride]); + assert(SI != IVUsesByStride.end() && "Stride doesn't exist!"); + StrengthReduceStridedIVUsers(SI->first, SI->second, L, HasOneStride); + } } // We're done analyzing this loop; release all the state we built up for it. @@ -1839,5 +1839,5 @@ bool LoopStrengthReduce::runOnLoop(Loop *L, LPPassManager &LPM) { DeleteTriviallyDeadInstructions(DeadInsts); } - return false; + return Changed; } diff --git a/test/CodeGen/X86/2008-07-07-DanglingDeadInsts.ll b/test/CodeGen/X86/2008-07-07-DanglingDeadInsts.ll new file mode 100644 index 00000000000..3586f87776a --- /dev/null +++ b/test/CodeGen/X86/2008-07-07-DanglingDeadInsts.ll @@ -0,0 +1,99 @@ +; RUN: llvm-as < %s | llc -mtriple=i386-apple-darwin9 + + %struct.ogg_stream_state = type { i8*, i32, i32, i32, i32*, i64*, i32, i32, i32, i32, [282 x i8], i32, i32, i32, i32, i32, i64, i64 } + %struct.res_state = type { i32, i32, i32, i32, float*, float*, i32, i32 } + %struct.vorbis_comment = type { i8**, i32*, i32, i8* } + +declare i32 @strlen(i8*) nounwind readonly + +define i32 @res_init(%struct.res_state* %state, i32 %channels, i32 %outfreq, i32 %infreq, i32 %op1, ...) nounwind { +entry: + br i1 false, label %bb95, label %bb + +bb: ; preds = %entry + br i1 false, label %bb95, label %bb24 + +bb24: ; preds = %bb + br i1 false, label %bb40.preheader, label %bb26 + +bb26: ; preds = %bb24 + ret i32 -1 + +bb40.preheader: ; preds = %bb24 + br i1 false, label %bb39, label %bb49.outer + +bb39: ; preds = %bb39, %bb40.preheader + shl i32 0, 1 ; :0 [#uses=0] + br i1 false, label %bb39, label %bb49.outer + +bb49.outer: ; preds = %bb39, %bb40.preheader + getelementptr %struct.res_state* %state, i32 0, i32 3 ; :1 [#uses=0] + getelementptr %struct.res_state* %state, i32 0, i32 7 ; :2 [#uses=0] + %base10.1 = select i1 false, float* null, float* null ; [#uses=1] + br label %bb74 + +bb69: ; preds = %bb74 + br label %bb71 + +bb71: ; preds = %bb74, %bb69 + store float 0.000000e+00, float* null, align 4 + add i32 0, 1 ; :3 [#uses=1] + %indvar.next137 = add i32 %indvar136, 1 ; [#uses=1] + br i1 false, label %bb74, label %bb73 + +bb73: ; preds = %bb71 + %.rec = add i32 %base10.2.ph.rec, 1 ; [#uses=2] + getelementptr float* %base10.1, i32 %.rec ; :4 [#uses=1] + br label %bb74 + +bb74: ; preds = %bb73, %bb71, %bb49.outer + %N13.1.ph = phi i32 [ 0, %bb49.outer ], [ 0, %bb73 ], [ %N13.1.ph, %bb71 ] ; [#uses=1] + %dest12.2.ph = phi float* [ null, %bb49.outer ], [ %4, %bb73 ], [ %dest12.2.ph, %bb71 ] ; [#uses=1] + %x8.0.ph = phi i32 [ 0, %bb49.outer ], [ %3, %bb73 ], [ %x8.0.ph, %bb71 ] ; [#uses=1] + %base10.2.ph.rec = phi i32 [ 0, %bb49.outer ], [ %.rec, %bb73 ], [ %base10.2.ph.rec, %bb71 ] ; [#uses=2] + %indvar136 = phi i32 [ %indvar.next137, %bb71 ], [ 0, %bb73 ], [ 0, %bb49.outer ] ; [#uses=1] + br i1 false, label %bb71, label %bb69 + +bb95: ; preds = %bb, %entry + ret i32 -1 +} + +define i32 @read_resampled(i8* %d, float** %buffer, i32 %samples) nounwind { +entry: + br i1 false, label %bb17.preheader, label %bb30 + +bb17.preheader: ; preds = %entry + load i32* null, align 4 ; :0 [#uses=0] + br label %bb16 + +bb16: ; preds = %bb16, %bb17.preheader + %i1.036 = phi i32 [ 0, %bb17.preheader ], [ %1, %bb16 ] ; [#uses=1] + add i32 %i1.036, 1 ; :1 [#uses=2] + icmp ult i32 %1, 0 ; :2 [#uses=0] + br label %bb16 + +bb30: ; preds = %entry + ret i32 0 +} + +define i32 @ogg_stream_reset_serialno(%struct.ogg_stream_state* %os, i32 %serialno) nounwind { +entry: + unreachable +} + +define void @vorbis_lsp_to_curve(float* %curve, i32* %map, i32 %n, i32 %ln, float* %lsp, i32 %m, float %amp, float %ampoffset) nounwind { +entry: + unreachable +} + +define i32 @vorbis_comment_query_count(%struct.vorbis_comment* %vc, i8* %tag) nounwind { +entry: + %strlen = call i32 @strlen( i8* null ) ; [#uses=1] + %endptr = getelementptr i8* null, i32 %strlen ; [#uses=0] + unreachable +} + +define fastcc i32 @push(%struct.res_state* %state, float* %pool, i32* %poolfill, i32* %offset, float* %dest, i32 %dststep, float* %source, i32 %srcstep, i32 %srclen) nounwind { +entry: + unreachable +}