[llvm-commits] Initial patch for FMA4 support

[Bruno Cardoso Lopes]

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