[llvm] r261430 - When MemoryDependenceAnalysis hits a CFG with many transparent blocks,

[Hal Finkel via llvm-commits]

----- Original Message -----
> From: "Tom Stellard" <tom at stellard.net>
> To: "Philip Reames" <listmail at philipreames.com>, hfinkel at anl.gov
> Cc: "thomas stellard" <thomas.stellard at amd.com>, "llvm-commits" <llvm-commits at lists.llvm.org>
> Sent: Monday, May 30, 2016 4:25:05 PM
> Subject: Re: [llvm] r261430 - When MemoryDependenceAnalysis hits a CFG with many transparent blocks,
> 
> On Tue, May 03, 2016 at 08:38:28PM -0700, Philip Reames via
> llvm-commits wrote:
> > 
> > 
> > On 05/01/2016 11:44 AM, Joerg Sonnenberger via llvm-commits wrote:
> > >On Mon, Feb 22, 2016 at 11:03:09AM -0800, Hans Wennborg wrote:
> > >>>>>New Revision: 261430
> > >>>>>
> > >>>>>URL: http://llvm.org/viewvc/llvm-project?rev=261430&view=rev
> > >>>>>Log:
> > >>>>>When MemoryDependenceAnalysis hits a CFG with many transparent
> > >>>>>blocks,
> > >>>>>the algorithm easily degrades into quadratic memory and time
> > >>>>>complexity.
> > >>>>>The easiest example is a long chain of BBs that don't
> > >>>>>otherwise use a
> > >>>>>location. The caching will add an entry for every intermediate
> > >>>>>block and
> > >>>>>limiting the number of results doesn't help as no results are
> > >>>>>produced
> > >>>>>until a definition is found.
> > >>>>>
> > >>>>>Introduce a limit similar to the existing
> > >>>>>instructions-per-block limit.
> > >>>>>This limit counts the total number of blocks checked. If the
> > >>>>>limit is
> > >>>>>reached, entries are considered unknown. The initial value is
> > >>>>>1000,
> > >>>>>which avoids regressions for normal sized functions while
> > >>>>>still
> > >>>>>limiting edge cases to reasnable memory consumption and
> > >>>>>execution time.
> > >>>>>
> > >>>>>Differential Revision: http://reviews.llvm.org/D16123
> > >>>>Hello Hans,
> > >>>>as mentioned on IRC the other day, I would like to see this
> > >>>>merged into
> > >>>>the 3.8 branch after the 3.8.0 release is cut. It fixes some
> > >>>>long
> > >>>>standing issues, but not regressions from earlier releases.
> > >>>I do not believe this should be merged into 3.8.
> > >>FWIW, I'm not considering this for 3.8.0 either, but what we said
> > >>on
> > >>IRC was that it might be a candidate for 3.8.1 if Tom finds it
> > >>appropriate. In any case, it should get properly reviewed first.
> > >Hi Tom,
> > >this has been in the tree for while now without any issues. I've
> > >been
> > >running with the merge for a while and it has helped to avoid the
> > >problems it was supposed to address. Can it be merged, please?
> > SGTM as well.
> > 
> > Philip
> 
> 
> Hi Hal,
> 
> I think this pass falls in the Alias Analysis category.  Is it OK
> to merge to the 3.8 branch: http://reviews.llvm.org/rL261430

Now, I normally consider it to be part of GVN ;)

> 
> Philip has already given the OK for this.

Okay, sounds good. It is okay with me too.

As a general note, this is a compile-time fix that introduces a new assert:

      assert((foundBlock || GotWorklistLimit) && "Current block not in cache?");

If we run into any problems with it, we'll need to back it out.

 -Hal

> 
> -Tom
> 
> 
> > _______________________________________________ >
> 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