[PATCH] D96110: [X86] Pass to transform tdpbf16ps intrinsics to scalar operation.

[Pengfei Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:318-319
+    // calculate idxa, idxb, idxc
+    // %eltc = extractelement <256 x i32> %vec.c.inner.phi, i16 %idxc
+    // %eltcf32 = bitcast i32 %eltc to float
+    // %elta = extractelement <256 x i32> %veca, i16 %idxa
----------------
Can we create vecC with <256 x float>?


================
Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:341
+    Value *EltC = B.CreateExtractElement(VecCPhi, IdxC);
+    Value *C_F32 = B.CreateBitCast(EltC, B.getFloatTy());
+    Value *EltA = B.CreateExtractElement(VecA, IdxA);
----------------
better to use EltCF32 or CF32


================
Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:348
+    int ShuffleMask[4] = {2, 0, 3, 1};
+    Value *A_V2F32 = B.CreateBitCast(
+        B.CreateShuffleVector(SubVecA, ZeroV2I16, makeArrayRef(ShuffleMask)),
----------------
ditto


================
Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:349
+    Value *A_V2F32 = B.CreateBitCast(
+        B.CreateShuffleVector(SubVecA, ZeroV2I16, makeArrayRef(ShuffleMask)),
+        V2F32Ty);
----------------
Better to define a variable for it and reuse.


================
Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:390-394
+  template <Intrinsic::ID IntrID,
+            typename = typename std::enable_if<
+                IntrID == Intrinsic::x86_tdpbssd_internal ||
+                IntrID == Intrinsic::x86_tdpbf16ps_internal>::type>
+  bool lowerTileDP(Instruction *TileDP);
----------------
Is it concise to use below?
```
template <Intrinsic::ID IntrID>
typename std::enable_if_t<
    IntrID == Intrinsic::x86_tdpbssd_internal ||
    IntrID == Intrinsic::x86_tdpbf16ps_internal, bool>
lowerTileDP(Instruction *TileDP);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96110