[llvm-commits] Initial patch for FMA4 support

Jan Sjodin jan_sjodin at yahoo.com
Wed Nov 23 13:24:46 PST 2011


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.


>+#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





More information about the llvm-commits mailing list