[llvm-commits] [llvm] r49419 - in /llvm/trunk: include/llvm/ include/llvm/Transforms/ lib/Transforms/Scalar/ test/Transforms/GVN/ test/Transforms/MemCpyOpt/ tools/llvm-ld/ tools/opt/

[Chris Lattner]

On Apr 9, 2008, at 1:23 AM, Owen Anderson wrote:

> Author: resistor
> Date: Wed Apr  9 03:23:16 2008
> New Revision: 49419
>
> URL: http://llvm.org/viewvc/llvm-project?rev=49419&view=rev
> Log:
> Factor a bunch of functionality related to memcpy and memset  
> transforms out of
> GVN and into its own pass.

Hi Owen,

> +//===- MemCpyOptimizer.cpp - Optimize use of memcpy and friends  
> -----------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open  
> Source
> +// License. See LICENSE.TXT for details.
> +//
> +// 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> +//
> +// This pass performs various transformations related to  
> eliminating memcpy
> +// calls, or transforming sets of stores into memset's.
> +//
> +// 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//

Ok.

> +#define DEBUG_TYPE "memcpyopt"
> +#include "llvm/Transforms/Scalar.h"
> +#include "llvm/BasicBlock.h"
> +#include "llvm/Constants.h"
> +#include "llvm/DerivedTypes.h"
> +#include "llvm/Function.h"
> +#include "llvm/IntrinsicInst.h"
> +#include "llvm/Instructions.h"
> +#include "llvm/ParameterAttributes.h"
> +#include "llvm/Value.h"
> +#include "llvm/ADT/DepthFirstIterator.h"
> +#include "llvm/ADT/SmallVector.h"
> +#include "llvm/ADT/Statistic.h"
> +#include "llvm/Analysis/Dominators.h"
> +#include "llvm/Analysis/AliasAnalysis.h"
> +#include "llvm/Analysis/MemoryDependenceAnalysis.h"
> +#include "llvm/Support/CFG.h"
> +#include "llvm/Support/CommandLine.h"
> +#include "llvm/Support/Compiler.h"
> +#include "llvm/Support/Debug.h"
> +#include "llvm/Support/GetElementPtrTypeIterator.h"
> +#include "llvm/Target/TargetData.h"
> +#include <list>

Please prune down this list, and the list in GVN.cpp.  BasicBlock.h  
and Value.h aren't needed, perhaps others as well.

>
> +using namespace llvm;
> +
> +STATISTIC(NumMemCpyInstr, "Number of memcpy instructions deleted");
> +STATISTIC(NumMemSetInfer, "Number of memsets inferred");
> +
> +namespace {
> +  cl::opt<bool>
> +  FormMemSet("form-memset-from-stores",
> +             cl::desc("Transform straight-line stores to memsets"),
> +             cl::init(true), cl::Hidden);
> +}

I think this can be removed now.  If Evan doesn't like it, he can  
complain ;-)

> +/// processInstruction - When calculating availability, handle an  
> instruction
> +/// by inserting it into the appropriate sets
> +bool MemCpyOpt::processInstruction(Instruction *I,
> +                                   SmallVectorImpl<Instruction*>  
> &toErase) {

This can be merged into iterateOnFunction

>
> +  if (StoreInst *SI = dyn_cast<StoreInst>(I))
> +    return processStore(SI, toErase);
> +
> +  if (MemCpyInst* M = dyn_cast<MemCpyInst>(I)) {
> +    MemoryDependenceAnalysis& MD =  
> getAnalysis<MemoryDependenceAnalysis>();
> +
> +    // The are two possible optimizations we can do for memcpy:
> +    //   a) memcpy-memcpy xform which exposes redundance for DSE
> +    //   b) call-memcpy xform for return slot optimization
> +    Instruction* dep = MD.getDependency(M);
> +    if (dep == MemoryDependenceAnalysis::None ||
> +        dep == MemoryDependenceAnalysis::NonLocal)
> +      return false;
> +    if (MemCpyInst *MemCpy = dyn_cast<MemCpyInst>(dep))
> +      return processMemCpy(M, MemCpy, toErase);
> +    if (CallInst* C = dyn_cast<CallInst>(dep))
> +      return performCallSlotOptzn(M, C, toErase);
> +    return false;

This should go into processMemCpy, to make an analog of processStore.

> +// MemCpyOpt::iterateOnFunction - Executes one iteration of GVN
> +bool MemCpyOpt::iterateOnFunction(Function &F) {
> +  bool changed_function = false;
> +
> +  DominatorTree &DT = getAnalysis<DominatorTree>();
> +
> +  SmallVector<Instruction*, 8> toErase;
> +
> +  // Top-down walk of the dominator tree
> +  for (df_iterator<DomTreeNode*> DI = df_begin(DT.getRootNode()),
> +         E = df_end(DT.getRootNode()); DI != E; ++DI) {

I don't think there is any reason for this code to walk the function  
in DF order, just walk in normal BB order.

>
> +
> +    BasicBlock* BB = DI->getBlock();
> +    for (BasicBlock::iterator BI = BB->begin(), BE = BB->end();
> +         BI != BE;) {
> +      changed_function |= processInstruction(BI, toErase);
> +      if (toErase.empty()) {
> +        ++BI;
> +        continue;
> +      }
> +
> +      // If we need some instructions deleted, do it now.
> +      NumMemCpyInstr += toErase.size();
> +
> +      // 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();

Can this deletion logic be simplified?  The memset and store  
simplifiers can delete stuff in place, right?

-Chris