[PATCH] D105263: [X86] AVX512FP16 instructions enabling 1/6

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke added inline comments.


================
Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:82
+  PatFrags ScalarIntMemFrags = !if (!eq (EltTypeName, "f16"),
+                                           !cast<PatFrags>("sse_load_f16"),
+                               !if (!eq (EltTypeName, "f32"),
----------------
indent


================
Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:3878
+}
+let Predicates = [HasFP16, HasVLX] in {
+  def : Pat<(v16f16 (vselect VK16WM:$mask, (v16f16 VR256X:$src1), (v16f16 VR256X:$src0))),
----------------
Not sure this can be merged to 512 version load/store pattern with muticlass by abstract type info.


================
Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:4159
+defm VMOVSHZ : avx512_move_scalar<"vmovsh", X86Movsh, X86vzload16, f16x_info,
+                                  [HasFP16]>,
+                                  VEX_LIG, T_MAP5XS, EVEX_CD8<16, CD8VT1>;
----------------
Why there is no OptForSize for vmovsh?


================
Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:4478
+  let Predicates = [HasFP16] in {
+    def VMOVSHZrr_REV: AVX512<0x11, MRMDestReg, (outs VR128X:$dst),
+        (ins VR128X:$src1, VR128X:$src2),
----------------
Sorry, I forgot what REV stand for. Do you know it?
Is this just encoding difference for register operand compared with VMOVSHZrr? What is it used for?


================
Comment at: llvm/lib/Target/X86/X86RegisterInfo.td:570
 def VR64: RegisterClass<"X86", [x86mmx], 64, (sequence "MM%u", 0, 7)>;
-def VR128 : RegisterClass<"X86", [v4f32, v2f64, v16i8, v8i16, v4i32, v2i64, f128],
+def VR128 : RegisterClass<"X86", [v4f32, v2f64, v8f16, v16i8, v8i16, v4i32, v2i64, f128],
                           128, (add FR32)>;
----------------
Given there is only EVEX instructions for fp16, is it necessary to add f16 type to it?


================
Comment at: llvm/lib/Target/X86/X86RegisterInfo.td:572
                           128, (add FR32)>;
-def VR256 : RegisterClass<"X86", [v8f32, v4f64, v32i8, v16i16, v8i32, v4i64],
+def VR256 : RegisterClass<"X86", [v8f32, v4f64, v16f16, v32i8, v16i16, v8i32, v4i64],
                           256, (sequence "YMM%u", 0, 15)>;
----------------
Ditto.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105263/new/

https://reviews.llvm.org/D105263