[PATCH] D84420: [AMDGPU] Add v3f16/v3i16 support to SDag

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp:961-962
+                          TLI.getTypeToTransformTo(*DAG.getContext(),
+                                                   Results[i].getValueType()) &&
+                      "Invalid type for widened vector");
+    if (IsWidened)
----------------
It looks like you intended this to be an assert?


================
Comment at: llvm/lib/Target/AMDGPU/BUFInstructions.td:1210-1220
+class MUBUF_LoadIntrinsicPatFragSize6<SDPatternOperator name> : PatFrag <
+  (ops node:$rsrc, node:$vindex, node:$voffset, node:$soffset, node:$offset,
+            node:$auxiliary, node:$todo), // TODO
+  (name node:$rsrc, node:$vindex, node:$voffset, node:$soffset, node:$offset,
+            node:$auxiliary, node:$todo) // TODO
+> {
+  let PredicateCode = [{
----------------
The boilerplate could be nicer and treated more uniformly. What you're really doing is defining an element vector extload/truncstore on intrinsics.

Can you move this to SIInstrInfo.td and have it follow along with something like truncstore_* or atomic nodes?

Something like:
class mubuf_intrinsic_load_ : PatFrag() ...
def mubuf_intrinsic_load_4 ..
def mubuf_intrinsic_load_6
def mubuf_intrinsic_load_8
...



================
Comment at: llvm/lib/Target/AMDGPU/BUFInstructions.td:1892-1895
+  let PredicateCode = [{
+    auto* MemNode = dyn_cast<MemSDNode>(N);
+    return MemNode && MemNode->getMemOperand()->getSize() == 6;
+  }];
----------------
It shouldn't be necessary to use predicate code. I think you can set MemoryVT which will also work for GlobalSel


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84420