[Patch] GVN fold conditional-branch on-the-fly

[Hal Finkel]

----- Original Message -----
> >
> >>>> GVN is getting more and more complicated,
> >>>
> >>> GVN is almost always complicated:-)
> >>
> >> It's really not.  LLVM's GVN is not really a GVN.  It's really a
> >> very
> >> complicated set of interleaved analysis and optimization that
> >> happens
> >> to include some amount of value numbering.
> >
> > LLVM GVN is a old-style GVN + some PRE.  But for the PRE part, I
> > don't
> > think LLVM GVN is complicated at all.
> 
> No, it's not. It is a very simple PRE that tries to catch a few
> cases.
> >
> >
> >> It is actually becoming
> >> more and more like GCC's old dominator optimization every day - a
> >> grab
> >> bag of optimizations that all fall under some abstract notion of
> >> value based redundancy elimination, but aren't really the same,
> >> and
> >> are not unified in any real way.  This is okay if it was not a
> >> compile
> >> time sink, but it's a huge one now, and adding stuff like this
> >> only
> >> makes it worse.
> >>
> >>
> >>>> and is already a compile time sink.
> >>>
> >>> Alias analysis bear the blame, It remember alias analysis account
> >>> for
> >>> 1/3+
> >>> of GVN compile-time.
> >>
> >> This is directly because of the algorithm in GVN and how it works.
> >
> > I don't think so.  I believe the culprit is lacking a sparse way to
> > represent
> > mem-dep -- when GVN try to figure out a the set of mem-opt a
> > particular
> > load/store
> > deps on, the mem-dep search all over the place.
> I disagree. As I said, the reason GVN's memory analysis is slow is
> simply because of how it uses memdep, which is unnecessarily slow and
> expensive.
> 
> It's true that a sparse representation would fit better with the
> current usage pattern, but as I mentioned in a later message, the
> current usage pattern is probably not actually a good thing.
> 
> As for a sparse representation, you can't sparsely and accurately
> represent mem-dep at the same time in a way that satisfies a lot of
> clients.  GCC learned this the hard way, and we tried for something
> like 8 years until we settled on what is there now, where clients are
> expected to use the sparse representation to get the nearest
> "possible" memory dependence, and then further disambiguate.
> 
> Computing this representation is actually fairly expensive to do
> well,
> and without a client past GVN, i'm not sure it would be worth it.
> 
> >
> >
> >> My suggestion would be to sit down and design something that
> >> actually
> >> covers the case we want, instead of just adding more stuff to GVN
> >> as
> >> it exists now, trying to get one oddball case at a time.
> >> This is going to have to be done at some point, and while I
> >> understand
> >> you don't want to have to be the one to do it, and I certainly
> >> can't
> >> make you, I can tell you that doing something like this is making
> >> the
> >> eventual job a bunch harder.
> >
> > Sound like I have to ditch this patch. I don't think I can come up
> > a better
> > way, considering the combination of what we have today.
> >
> 
> That, of course, is up to you.  I am just offering an opinion. I'd
> love to hear what others think.

I think that we should evaluate this change just like any other: using a cost-benefit analysis of performance vs. compile-time impact averaged over the test suite (and any other code bases we care about). As a practical matter, we should not hold all GVN-related improvements hostage to competition of a GVN rewrite on which no one is actively working.

> 
> > How soon will your new GVN available.
> 
> I'm not actively working on it right now.
> > Dose it tackle this problem as well?
> 
> It does, but what I wrote was essentially a prototype that worked.
> Even in the end it was faster and caught all these cases (and passed
> tests), it was a mess.
> Basically, someone needs to sit down and write a clean version :)

Perhaps this is the wrong thread for this, but I took a quick look at your gvn-rewrite github repo, and can you elaborate on how the cleaner version would differ from the current prototype? Obviously there are still some TODOs and FIXMEs in there, but if the code is functionally correct, and better than what we currently have, why not start the review process? There are some things that may need to be fixed before it goes upstream (like better handling memory intrinsics), but things like replacing the old GVN::propagateEquality implementation from the current GVN with something new could certainly wait: it seems no worse than what we currently have. Please don't take this the wrong way, but I'd like to avoid the classic situation where the perfect becomes the enemy of the good.

 -Hal

> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory