[PATCH] Add peephole optimization to use LEA instructions of Intel Atom

Nadav Rotem nrotem at apple.com
Tue Apr 23 15:59:24 PDT 2013


Hi Preston, 

You don't need to update the LiveVariable analysis because it is only alive during the register allocation passes (PHIElim, TwoAddr, etc) and not after register allocation.  

Please document your code properly (capital letters, periods at the end of lines) and document it (doxygen above declarations, etc). Also, you have some code commented out. You don't need to change X86InstrInfo.cpp (if you care about the header ordering you can commit it as a separate patch). 

Nadav


On Apr 23, 2013, at 9:48 AM, "Gurd, Preston" <preston.gurd at intel.com> wrote:

> Thanks for your comments, Nadav.
>  
> I just posted a revised diff, which moves the changes which the patch made to convertToThreeAddress into a separate function internal to the FixupLEAPass.
>  
> Since the convertToThreeAddress function handles all of the possibilities for converting an instruction to an LEA (add, sub, inc, dec, mul, shift, etc.), it seemed that it would be better to use existing code than to duplicate it.
>  
> While I agree that it would be desirable in theory to merge the fixup LEA pass into pad short functions pass, I think that in practice the two passes work so very differently that a combined pass would be overly complex and difficult to read and maintain.
>  
> Preston
>  
> From: Nadav Rotem [mailto:nrotem at apple.com] 
> Sent: Tuesday, April 16, 2013 8:06 PM
> To: Gurd, Preston
> Cc: reviews+D660+public+a10049d8ed57ba7c at llvm-reviews.chandlerc.com; Du Toit, Stefanus; llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH] Add peephole optimization to use LEA instructions of Intel Atom
>  
> Hi Preston, 
>  
> I understand that your in-order processor is affected by the scheduling decision and you need some peepholes after RA/scheduling.  But please don't change X86InstrInfo::convertToThreeAddress because this function is meant to be called before register allocation. Your change adds complexity to this function and does not help the non FixupLeaPass users.  Also, you really don't reuse much code from convertToThreeAddress. You don't need all of the code that handles the INCs and the 64bit instructions and the Shifts, etc. What you really need is a few peepholes inside your own pass. 
>  
> Also, maybe you should merge it with your other in-order ATOM pass that pads small functions that don't get inlined. It looks like all of these optimizations fall into the same category and I am not sure that we need lots of different passes for every micro architectural issue. 
>  
> Thanks,
> Nadav 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130423/4a318c92/attachment.html>


More information about the llvm-commits mailing list