[llvm-commits] [llvm] r122659 - in /llvm/trunk: lib/Transforms/Scalar/LoopIdiomRecognize.cpp test/Transforms/LoopIdiom/basic.ll

Frits van Bommel fvbommel at gmail.com
Sat Jan 1 16:54:45 PST 2011


On Sat, Jan 1, 2011 at 8:39 PM, Chris Lattner <sabre at nondot.org> wrote:
> +/// mayLoopModRefLocation - Return true if the specified loop might do a load or
> +/// store to the same location that the specified store could store to, which is
> +/// a loop-strided access.
> +static bool mayLoopModRefLocation(StoreInst *SI, Loop *L, AliasAnalysis &AA) {
> +  // Get the location that may be stored across the loop.  Since the access is
> +  // strided positively through memory, we say that the modified location starts
> +  // at the pointer and has infinite size.
> +  // TODO: Could improve this for constant trip-count loops.
> +  AliasAnalysis::Location StoreLoc =
> +    AliasAnalysis::Location(SI->getPointerOperand());
> +
> +  for (Loop::block_iterator BI = L->block_begin(), E = L->block_end(); BI != E;
> +       ++BI)
> +    for (BasicBlock::iterator I = (*BI)->begin(), E = (*BI)->end(); I != E; ++I)
> +      if (AA.getModRefInfo(I, StoreLoc) != AliasAnalysis::NoModRef)
> +        return true;

Wouldn't it be cleaner to add an 'I != SI &&' to the condition here
instead of temporarily removing SI from the block in before calling
this?
(The name of the function would probably have to be changed, but I
think it's currently a bit confusing anyway: the comment implies SI is
in the loop but the function checks whether any instruction in the
loop modrefs the pointer operand, which would *normally* trivially be
true...)

Of course, checking that extra condition for every instruction in the
loop is probably less efficient, which I'm guessing is the reason you
did it this way?

At the very least, it'd be nice to update the comment so it mentions
the store should be temporarily removed before calling this.

> +
> +  return false;
> +}
> +
>  /// processLoopStoreOfSplatValue - We see a strided store of a memsetable value.
>  /// If we can transform this into a memset in the loop preheader, do so.
>  bool LoopIdiomRecognize::
>  processLoopStoreOfSplatValue(StoreInst *SI, unsigned StoreSize,
>                              Value *SplatValue,
>                              const SCEVAddRecExpr *Ev, const SCEV *BECount) {
> +  // Temporarily remove the store from the loop, to avoid the mod/ref query from
> +  // seeing it.
> +  Instruction *InstAfterStore = ++BasicBlock::iterator(SI);
> +  SI->removeFromParent();
> +
>   // Okay, we have a strided store "p[i]" of a splattable value.  We can turn
>   // this into a memset in the loop preheader now if we want.  However, this
>   // would be unsafe to do if there is anything else in the loop that may read
>   // or write to the aliased location.  Check for an alias.
> +  bool Unsafe=mayLoopModRefLocation(SI, CurLoop, getAnalysis<AliasAnalysis>());
> +
> +  SI->insertBefore(InstAfterStore);




More information about the llvm-commits mailing list