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

Hal Finkel via llvm-commits llvm-commits at lists.llvm.org
Thu May 26 13:57:57 PDT 2016


----- Original Message -----

> From: "David Majnemer via llvm-commits" <llvm-commits at lists.llvm.org>
> To: "Eli Friedman" <eli.friedman at gmail.com>
> Cc: "llvm-commits" <llvm-commits at lists.llvm.org>
> Sent: Thursday, May 26, 2016 3:33:13 PM
> Subject: Re: [llvm] r270892 - [MemCpyOpt] Don't perform callslot
> optimization across may-throw calls

> On Thu, May 26, 2016 at 1:21 PM, Eli Friedman <
> eli.friedman at gmail.com > wrote:

> > 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.
> 
> Ah, great point! IMO, we should model longjmp as a "throw" as the
> optimizer has to worry about it in the same ways.
Don't we model it with some kind of does-not-return annotation? 

-Hal 

> > -Eli
> 

> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

-- 

Hal Finkel 
Assistant Computational Scientist 
Leadership Computing Facility 
Argonne National Laboratory 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160526/6456e051/attachment.html>


More information about the llvm-commits mailing list