[llvm-commits] Initial patch for FMA4 support

[Bruno Cardoso Lopes]

Committed r145133

On Wed, Nov 23, 2011 at 8:13 PM, Jan Sjodin <jan_sjodin at yahoo.com> wrote:
> Here is the updated patch.
>
> - Jan
>
>
>
> ----- Original Message -----
>> From: Bruno Cardoso Lopes <bruno.cardoso at gmail.com>
>> To: Jan Sjodin <jan_sjodin at yahoo.com>
>> Cc: "llvm-commits at cs.uiuc.edu" <llvm-commits at cs.uiuc.edu>
>> Sent: Wednesday, November 23, 2011 4:30 PM
>> Subject: Re: [llvm-commits] Initial patch for FMA4 support
>>
>> On Wed, Nov 23, 2011 at 7:24 PM, Jan Sjodin <jan_sjodin at yahoo.com> wrote:
>>>  Thanks Bruno for the quick review.
>>>
>>>
>>>> There's no specific reason in this case to have the "Int"
>> forms:
>>>
>>>>
>>>> +multiclass fma4s_Int<bits<8> opc, string OpcodeStr, Intrinsic
>> Int,
>>>> +        PatFrag ld_frag> {
>>>> ...
>>>> +defm Int_VFMADDSD4  : fma4s_Int<0x6B, "vfmaddsd",
>>>> int_x86_fma4_vfmadd_sd, alignedloadv2f64>;
>>>>
>>>> Instead, just write patterns to match the intrinsics with the
>>>> previously defined FMA4 instructions.
>>>
>>>
>>>  Okay, will do.
>>>
>>>
>>>>
>>>>        if (HasVEX_4V)
>>>>          ++FirstMemOp;// Skip the register source (which is encoded in
>>>> VEX_VVVV).
>>>> +      if (HasXOP_W)
>>>> +        ++FirstMemOp;// Skip the register source (which is encoded in
>> I8IMM).
>>>>
>>>> Use " if (HasVEX_4V || HasXOP_W)" and update the comment
>> properly.
>>>
>>>
>>>  It actually increments twice if both VEX_4V and HasXOP_W are true, not by
>> one.
>>>
>>>  I could update the comment to make it more clear e.g.:
>>>
>>>  // Skip second register source (which is encoded in I8IMM)
>>>
>>>  If you want it written a different way let me know.
>>
>> Oops. No need, it's fine this way! :)
>>
>>>
>>>> +#include "llvm/Support/Debug.h"
>>>>
>>>> Are you sure you need this?
>>>
>>>  No, forgot to remove, will fix.
>>>
>>>
>>>>
>>>> +    } else {
>>>> +      CurOp += AddrOperands + 1;
>>>> +      if (HasVEX_4VOp3)
>>>> +    ++CurOp;
>>>> +    }
>>>>
>>>> "++CurOp" lacks proper indentation!
>>>
>>>  Will fix.
>>>
>>>
>>>  - Jan
>>>
>>>
>>
>>
>>
>> --
>> Bruno Cardoso Lopes
>> http://www.brunocardoso.cc
>>



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc