[llvm-commits] [llvm] r48940 - /llvm/trunk/lib/Transforms/Scalar/GVN.cpp

[Owen Anderson]

Chris,

What's the threshold for splitting this and other memory optzns into a  
separate pass?  The core functionality of GVN is starting to get  
overwhelmed by the volume of extra things we've tacked onto it?

--Owen

On Mar 29, 2008, at 12:15 AM, Chris Lattner wrote:

> Author: lattner
> Date: Sat Mar 29 00:15:47 2008
> New Revision: 48940
>
> URL: http://llvm.org/viewvc/llvm-project?rev=48940&view=rev
> Log:
> change iterator invalidation avoidance to just move the iterator  
> backward
> when something changes, instead of moving forward.  This allows us to
> simplify memset lowering, inserting the memset at the end of the  
> range of
> stuff we're touching instead of at the start.
>
> This, in turn, allows us to make use of the addressing instructions  
> already
> used in the function instead of inserting our own.  For example, we  
> now
> codegen:
>
> 	%tmp41 = getelementptr [8 x i8]* %ref_idx, i32 0, i32 0		; <i8*>  
> [#uses=2]
> 	call void @llvm.memset.i64( i8* %tmp41, i8 -1, i64 8, i32 1 )
>
> instead of:
>
> 	%tmp20 = getelementptr [8 x i8]* %ref_idx, i32 0, i32 7		; <i8*>  
> [#uses=1]
> 	%ptroffset = getelementptr i8* %tmp20, i64 -7		; <i8*> [#uses=1]
> 	call void @llvm.memset.i64( i8* %ptroffset, i8 -1, i64 8, i32 1 )
>
>
> Modified:
>    llvm/trunk/lib/Transforms/Scalar/GVN.cpp
>
> Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=48940&r1=48939&r2=48940&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Sat Mar 29 00:15:47 2008
> @@ -1232,8 +1232,10 @@
>   // See if the range extends the start of the range.  In this case,  
> it couldn't
>   // possibly cause it to join the prior range, because otherwise we  
> would have
>   // stopped on *it*.
> -  if (Start < I->Start)
> +  if (Start < I->Start) {
>     I->Start = Start;
> +    I->StartPtr = SI->getPointerOperand();
> +  }
>
>   // Now we know that Start <= I->End and Start >= I->Start (so the  
> startpoint
>   // is in or right at the end of I), and that End >= I->Start.   
> Extend I out to
> @@ -1345,18 +1347,17 @@
>       continue;
>
>     // Otherwise, we do want to transform this!  Create a new  
> memset.  We put
> -    // the memset right after the first store that we found in this  
> block.  This
> -    // ensures that the caller will increment the iterator to the  
> memset before
> -    // it deletes all the stores.
> -    BasicBlock::iterator InsertPt = SI; ++InsertPt;
> +    // the memset right before the first instruction that isn't  
> part of this
> +    // memset block.  This ensure that the memset is dominated by  
> any addressing
> +    // instruction needed by the start of the block.
> +    BasicBlock::iterator InsertPt = BI;
>
>     if (MemSetF == 0)
>       MemSetF = Intrinsic::getDeclaration(SI->getParent()->getParent()
>                                           ->getParent(),  
> Intrinsic::memset_i64);
>
> -    // StartPtr may not dominate the starting point.  Instead of  
> using it, base
> -    // the destination pointer off the input to the first store in  
> the block.
> -    StartPtr = SI->getPointerOperand();
> +    // Get the starting pointer of the block.
> +    StartPtr = Range.StartPtr;
>
>     // Cast the start ptr to be i8* as memset requires.
>     const Type *i8Ptr = PointerType::getUnqual(Type::Int8Ty);
> @@ -1364,12 +1365,6 @@
>       StartPtr = new BitCastInst(StartPtr, i8Ptr, StartPtr- 
> >getNameStart(),
>                                  InsertPt);
>
> -    // Offset the pointer if needed.
> -    if (Range.Start)
> -      StartPtr = new GetElementPtrInst(StartPtr,  
> ConstantInt::get(Type::Int64Ty,
> -                                                                   
> Range.Start),
> -                                       "ptroffset", InsertPt);
> -
>     Value *Ops[] = {
>       StartPtr, ByteVal,   // Start, value
>       ConstantInt::get(Type::Int64Ty, Range.End-Range.Start),  // size
> @@ -1695,7 +1690,7 @@
>
>   DominatorTree &DT = getAnalysis<DominatorTree>();
>
> -  SmallVector<Instruction*, 4> toErase;
> +  SmallVector<Instruction*, 8> toErase;
>   DenseMap<Value*, LoadInst*> lastSeenLoad;
>
>   // Top-down walk of the dominator tree
> @@ -1713,19 +1708,31 @@
>       currAvail = availableOut[DI->getIDom()->getBlock()];
>
>     for (BasicBlock::iterator BI = BB->begin(), BE = BB->end();
> -         BI != BE; ) {
> +         BI != BE;) {
>       changed_function |= processInstruction(BI, currAvail,
>                                              lastSeenLoad, toErase);
> +      if (toErase.empty()) {
> +        ++BI;
> +        continue;
> +      }
>
> +      // If we need some instructions deleted, do it now.
>       NumGVNInstr += toErase.size();
>
> -      // Avoid iterator invalidation
> -      ++BI;
> +      // Avoid iterator invalidation.
> +      bool AtStart = BI == BB->begin();
> +      if (!AtStart)
> +        --BI;
>
>       for (SmallVector<Instruction*, 4>::iterator I = toErase.begin(),
>            E = toErase.end(); I != E; ++I)
>         (*I)->eraseFromParent();
>
> +      if (AtStart)
> +        BI = BB->begin();
> +      else
> +        ++BI;
> +
>       toErase.clear();
>     }
>   }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2555 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20080329/cf281343/attachment.bin>