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

Daniel Berlin dberlin at dberlin.org
Sat Sep 7 16:14:37 PDT 2013


>> >
>>
>> 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).

Such costs need to include the cost of actually maintaining the code, IMHO :)
Anything else will just set you down a path of death by a thousand needles
> 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.

I certainly wouldn't suggest that, but you do have to draw a line in
the sand somewhere, or things will only ever get worse.

>>
>> > 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?

The scalar expression handling is great. The memory expression
handling needs some rethinking and probably separation.
An analysis needs to be made of the benefits of doing some
simplifications on the fly.

Some of the engineering and data structure usage is ugly.  The overall
structure was built against GVN, one piece at a time.  Parts of the
old GVN were used and replaced as i went along.  Some of it is fairly
fragile as a result.

Really, someone (me, or someone else) needs to take the things learned
from it, and use it to write a GVN.
You could not, for example, spend a weekend cleaning it up and declare victory.

>  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?

It's algorithmically correct at this point, but in the end, I was
doing the work to see how much it would take to do better than the
current GVN does, in a unified way.

In this respect, it was successful. It can catch roughly every case,
and in fact, even more than the current GVN. It is faster than the
current GVN.  It does so in a unified way, and completely separates
analysis and redundancy elimination.
But, like any research code, the process of productionizing it
essentially means one of two things:

1. Cleaning up what is there, making it l
or
2. Taking the knowledge gained from it and writing something anew.

I think #1 would be significantly more difficult than #2, and i think
we have existence proofs of this (I know chandler set some folks on
it).
For example, i probably would *not* do memory handling the way it does
it, i would still explore some other options.

A proper way to view this code would be as a peek into the mind of a
mad scientist attempting to unify our current GVN approach, who
succeeded in creating something that accomplished this goal.  It's not
any more horrible than research code you see elsewhere, but it is in
fact, research code.



More information about the llvm-commits mailing list