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

[Philip Reames]

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.

For code comments, please put this on phabricator.  I see a number of 
small issues which need addressed.

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?

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?

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 
> <mailto:chandlerc at google.com>> wrote:
>
>>
>> On Fri, May 2, 2014 at 4:44 PM, Gerolf Hoflehner 
>> <ghoflehner at apple.com <mailto: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/c8266d62/attachment.html>