[PATCH] [X86] Use ADD/SUB instead of INC/DEC for Silvermont

[Alexey Volkov]

I updated patch according to your comments.


2014-06-05 20:32 GMT+04:00 Nadav Rotem <nrotem at apple.com>:

>
> On Jun 5, 2014, at 9:01 AM, Alexey Volkov <avolkov.intel at gmail.com> wrote:
>
> Hi Nadav,
>
>
> 2014-06-05 9:33 GMT+04:00 Nadav Rotem <nrotem at apple.com>:
>
>> Hi Alexey,
>>
>> Can you please reduce the size of the testcase? It is really big.
>>
>  I attached patch with reduced testcase.
>
>>
>>
>> -    if (ConstantSDNode *C =
>> -        dyn_cast<ConstantSDNode>(ArithOp.getNode()->getOperand(1))) {
>> -      // An add of one will be selected as an INC.
>> -      if (C->getAPIntValue() == 1) {
>> -        Opcode = X86ISD::INC;
>> -        NumOperands = 1;
>> -        break;
>> -      }
>> +    if (!Subtarget->slowIncDec())
>> +      if (ConstantSDNode *C =
>> +          dyn_cast<ConstantSDNode>(ArithOp.getNode()->getOperand(1))) {
>> +        // An add of one will be selected as an INC.
>> +        if (C->getAPIntValue() == 1) {
>> +          Opcode = X86ISD::INC;
>> +          NumOperands = 1;
>> +          break;
>> +        }
>>
>> Can you please perform the slowIncDec check after the ‘&&’ operator
>> inside the second if?  This will help reducing the compile time and allow
>> you to change less code.
>>
> Checking slowIncDec couldn't be placed in condition with ConstantSDNode *C
> since it's a declaration statement inside if().
> Also slowIncDec field check should be cheaper than dyn_cast<> in if().
>
>
> So add another if inside.  I prefer that we change as little code as
> possible. Also, almost no x86 target will satisfy slowIncDec so this check
> will just make everybody else slower for no good reason. Just place the IF
> inside.
>
>
>> Will this work?
>> -def : Pat<(add GR16:$src, -1), (DEC16r    GR16:$src)>,
>> Requires<[Not64BitMode, NotSlowIncDec]>;
>>
> Yes, this will work. But I intentionally made changes with "let Predicates
> =" to get code more readable and structured.
>
>
> I think that adding another parameter and a comment above makes things
> more readable.
>
>
>> Everything else looks okay.
>>
>> Thanks,
>> Nadav
>>
>>
>> On Jun 2, 2014, at 7:28 AM, Alexey Volkov <avolkov.intel at gmail.com>
>> wrote:
>>
>> > <D3990.10015.patch>
>>
>>
>
>
> --
> Alexey Volkov
> Intel Corporation
>  <incdec.patch>
>
>
>


-- 
Alexey Volkov
Intel Corporation
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140606/67af95ce/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: incdec.patch
Type: application/octet-stream
Size: 8531 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140606/67af95ce/attachment.obj>