[llvm-commits] [PATCH] add-carray/sub-borrow optimization (CodeGen, X86)

[manman ren]

On Oct 31, 2012, at 1:51 PM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:

> Hi, Manman:
> 
>  You are right! Please find the revised patch in the attachment.
> 
>  In this revised patch, I factor out the non-trivial code and put it into a tiny function called MaterializeSETB().
> Now we can return early by calling this tiny function, and the dead code is deleted as well.

LGTM, please commit.

BTW, I thought early-return can remove some codes, but actually those codes can be removed regardless.
>>> If we return SETCC_CARRY directly, there is no need to create NewVal.


Thanks,
Manman


> 
> Thanks
> Shuxin
> 
> On 10/31/12 10:37 AM, manman ren wrote:
>> On Oct 31, 2012, at 10:30 AM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
>> 
>>> Hi, Manman:
>>> 
>>>   I read "coding guide" the other day. The document also suggests "return early" to make the logic easier.
>>> But it doesn't say on what condition the "return early" is appropriate. In this case, we have to duplicate
>>> non-trivial code in order to return early.  For sloppy engineers like me, maintaining duplication is difficult.
>>> I'd like to improve maintenance even at the cost of sacrificed readability.
>> We can probably get rid of
>>>>>> +      SDValue NewVal = DAG.getNode(X86ISD::SETCC, DL, N->getVTList(),
>>>>>> +                                   DAG.getConstant(CC, MVT::i8), EFLAGS);
>>>>>> +      N = NewVal.getNode ();
>> since SETCC will be replaced with a SETCC_CARRAY and N is not used in the followed if statement.
>> 
>> Otherwise LGTM.
>> 
>> Thanks,
>> 
>> 
> 
> <diff.patch>