[PATCH] [InstructionMerge - GVN] hoisting and sinking of equivalent memory instructions in diamonds

[Gerolf Hoflehner]

Thanks, Philip. Please see below.

On Jun 10, 2014, at 10:17 AM, Philip Reames <listmail at philipreames.com> wrote:

> Before anything else, the naming and documentation of this pass need improved.  I have no idea what "IM" stands for and the file level documentation doesn't say.  I finally got down to the pass registration and found "Instruction Merger".  This is far too vague to be useful.  
I’m open for good suggestions, otherwise go with instruction merger. When you know what it is it becomes less vague. This is similar to GVN etc.
> 
> For code comments, please put this on phabricator.  I see a number of small issues which need addressed. 

Alright, that sounds cool. Here is my first attempt at it: 
http://reviews.llvm.org/D4096


> 
> 
> This code seems like it duplicates lots of legality tests from other places in the code base.  I don't see obvious ommisions, but the code duplication is questionable.  Could you refactor out a single shared copy?

I know what you mean, but could not find good abstractions for a few consistency checks and decided to use the code as is. But if you have a specific proposal I’m happy to revisit.
> 
> More generally, there's a lot of shared logic which other passes which perform code hoisting and sinking (LICM for one).  Why does this need to be it's own pass rather than an extension to something existing?
It is a separate pass because there is no obvious way to hook it into an existing one. I tried a kind of piggyback in GVN, but agreed with Chandler that a separate pass is more appropriate. While there is a little duplication in logic it keeps passes independent and simple. And based on Chandler's claims below it should also have the nice side effect of making the review a snap for him ;-)
> 
> Philip
> 
> On 06/09/2014 08:42 PM, Gerolf Hoflehner wrote:
>> Alright, time has come to move on with this one. The instruction merge code is now a pass eagerly awaiting  expert review eyes.
>> 
>> Cheers
>> Gerolf
>> 
>> 
>> 
>> 
>> On May 2, 2014, at 5:13 PM, Chandler Carruth <chandlerc at google.com> wrote:
>> 
>>> 
>>> On Fri, May 2, 2014 at 4:44 PM, Gerolf Hoflehner <ghoflehner at apple.com> wrote:
>>> I can see the arguments pro independent pass, but why do you assert it will make the code much easier to review? The code is coherent as it is and won’t change whether it is  part of GVN or its own phase. 
>>> 
>>> Because then I don't have to even think about GVN? Anyways, it should be *much* easier to test.
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140610/11feced6/attachment.html>