[PATCH] D142782: [AMDGPU] Add basic support for extended i8 perm matching

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

Should ByteProvider really be BitProvider?



================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:9856
+  // depth
+  if (Depth >= 8)
+    return std::nullopt;
----------------
Why 8? Usually 6 is the one true recursion depth limit


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:9866-9869
+    auto LHS = calculateByteProvider(Op.getOperand(0), Index, Depth + 1,
+                                     StartingIndex);
+    auto RHS = calculateByteProvider(Op.getOperand(1), Index, Depth + 1,
+                                     StartingIndex);
----------------
Do the RHS calls first and short circuit the second call if the first failed


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:9955
+      return Op.getOpcode() == ISD::ZERO_EXTEND
+                 ? std::optional<ByteProvider<SDValue>>(
+                       ByteProvider<SDValue>::getConstantZero())
----------------
Do you really need the explicit std::optional?


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:10134
+      // vectorized ops, then conservatively do not attempt to combine.
+      for (auto VUse : OrUse->uses()) {
+        if (VUse->getOpcode() == ISD::BITCAST ||
----------------
llvm::any_of?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142782