[llvm-commits] [PATCH] Peephole optimization to remove redundant cmp instruction for X86

[Eric Christopher]

These could use some better block comments and perhaps names. The "update" doesn't really say what's going on and what you'd use this function for (other than a single use function in the code below).

-eric

On Jun 1, 2012, at 11:36 AM, Manman Ren <mren at apple.com> wrote:

> 
> I did some refactoring, pulling the big switch statement out to 3 short functions.
> 
> Thanks,
> <peephole_x862.patch>
> Manman
> 
> On May 31, 2012, at 8:42 AM, Manman Ren wrote:
> 
>> 
>> 
>> Thanks for reviewing.
>> 
>> Manman
>> 
>> On May 30, 2012, at 11:16 PM, Chandler Carruth wrote:
>> 
>>> Please ping within the same thread... Pretty sure we already talked about this patch on another thread, but hard to remember.
>>> 
>>> 
>> Yes, this is a re-work of a previous patch "[llvm-commits] [Patch] Optimize away a cmp instruction on X86".
>> There is a corresponding implementation for ARM in r156599.
>> 
>>> Anyways, I'm not an x86 backend expert, but from my limited experience, I have some concerns with the patch as currently implemented. At a high level, it seems overly focused on the 'sub' instruction. Given the model of the EFLAGS set by each instruction, could you turn this into a generic optimization which tries to remove 'cmp' instructions (which have no side-effects other than to set EFLAGS) by using those flags set by arbitrary preceding instructions?
>>> 
>> This optimization is trying to remove a redundant instruction when it sets EFLAGS only and EFLAGS has been set up correctly already.
>> When removing 'cmp' instruction, we have to make sure the previous instruction sets the EFLAGS in the same way as 'cmp'.
>> I know that "sub a, b" sets up EFLAGS in the same way as "cmp a, b", and there may be other pairs I don't know of.
>> 
>>> Also, I wonder if the backend should have a model for equivalent instruction forms where one does set EFLAGS and the other doesn't so that we can flip instructions back and forth as needed.
>>> 
>> For X86, I don't know equivalent instruction pairs where one set EFLAGS and the other does not, like sub and subs for ARM.
>>> Finally, I feel like a lot of the large switches over instructions could be simplified by querying the nature of the EFLAGS usage, or some other shared aspect embedded in the instruction via the tablegen file.
>>> 
>> It would be nice if we can get the condition code from a machine instruction, but I didn't find that interface for X86.
>> The large switch is to toggle the condition code of a machine instruction.
>>> 
>>> A minor style nit-pick: the coding guidelines recommend 'camelCase' function and method names, not 'CamelCase'.
>>> 
>>> 
>>> 
>>> On Wed, May 30, 2012 at 8:55 PM, Manman Ren <mren at apple.com> wrote:
>>> 
>>> PING
>>> 
>>> On May 30, 2012, at 11:30 AM, Manman Ren wrote:
>>> 
>>> >
>>> > Hi All,
>>> >
>>> > This patch optimizes away the redundant cmp instruction if there exists a sub instruction which produces the same EFLAGS.
>>> >
>>> > Please review and provide feedback.
>>> >
>>> > Thanks,
>>> > Manman
>>> >
>>> > <peephole_x86.patch>
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> 
>> 	
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits