[llvm] r270892 - [MemCpyOpt] Don't perform callslot optimization across may-throw calls

[Eli Friedman via llvm-commits]

On Thu, May 26, 2016 at 12:24 PM, David Majnemer via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: majnemer
> Date: Thu May 26 14:24:24 2016
> New Revision: 270892
>
> URL: http://llvm.org/viewvc/llvm-project?rev=270892&view=rev
> Log:
> [MemCpyOpt] Don't perform callslot optimization across may-throw calls
>
> An exception could prevent a store from occurring but MemCpyOpt's
> callslot optimization would fire anyway, causing the store to occur.
>
> This fixes PR27849.
>
> Added:
>     llvm/trunk/test/Transforms/MemCpyOpt/callslot_throw.ll
> Modified:
>     llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp
>     llvm/trunk/test/Transforms/MemCpyOpt/loadstore-sret.ll
>
> Modified: llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp?rev=270892&r1=270891&r2=270892&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp Thu May 26
> 14:24:24 2016
> @@ -496,7 +496,7 @@ static unsigned findCommonAlignment(cons
>
>  // This method try to lift a store instruction before position P.
>  // It will lift the store and its argument + that anything that
> -// lay alias with these.
> +// may alias with these.
>  // The method returns true if it was successful.
>  static bool moveUp(AliasAnalysis &AA, StoreInst *SI, Instruction *P) {
>    // If the store alias this position, early bail out.
> @@ -675,6 +675,8 @@ bool MemCpyOpt::processStore(StoreInst *
>        if (C) {
>          // Check that nothing touches the dest of the "copy" between
>          // the call and the store.
> +        Value *CpyDest = SI->getPointerOperand()->stripPointerCasts();
> +        bool CpyDestIsLocal = isa<AllocaInst>(CpyDest);
>          AliasAnalysis &AA =
> getAnalysis<AAResultsWrapperPass>().getAAResults();
>          MemoryLocation StoreLoc = MemoryLocation::get(SI);
>          for (BasicBlock::iterator I = --SI->getIterator(), E =
> C->getIterator();
> @@ -683,6 +685,12 @@ bool MemCpyOpt::processStore(StoreInst *
>              C = nullptr;
>              break;
>            }
> +          // The store to dest may never happen if an exception can be
> thrown
> +          // between the load and the store.
> +          if (I->mayThrow() && !CpyDestIsLocal) {
> +            C = nullptr;
> +            break;
> +          }
>          }
>        }
>
> @@ -815,6 +823,10 @@ bool MemCpyOpt::performCallSlotOptzn(Ins
>      if (destSize < srcSize)
>        return false;
>    } else if (Argument *A = dyn_cast<Argument>(cpyDest)) {
> +    // The store to dest may never happen if the call can throw.
> +    if (C->mayThrow())
> +      return false;
>

I'm not sure mayThrow is sufficient; it doesn't cover longjmp.  Maybe not
worth worrying about, though.

-Eli
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160526/9f3a5b45/attachment.html>