[PATCH] D130895: [RISCV] Make VL choosing for a splat-like VMV based on its users

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h:191
 
+static inline unsigned getMergeOpNum(const MCInstrDesc &Desc) {
+  const uint64_t TSFlags = Desc.TSFlags;
----------------
If you rebase, these have landed through other patches which needed them.  


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:2452
+    return SDValue{};
+  const MCInstrDesc &MCID = TII.get(Node->getMachineOpcode());
+  if (!RISCVII::hasVLOp(MCID.TSFlags))
----------------
JFYI, this looks suspicious.  The node operands of the SDNode and the MI are not guaranteed by construction to be the same.  Or at least if they are, we haven't adjusted other code under that assumption and asserted appropriately.  


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:2519
+  const RISCVInstrInfo &TII = *Subtarget->getInstrInfo();
+  for (SDNode::use_iterator UI = Node->use_begin(), UE = Node->use_end();
+       UI != UE; ++UI)
----------------
I request that you separate this patch into one which is one-use restricted, and then a second which adds the use walk.  The former patch would parallel what we do for e.g. insert_vector folds.  


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:2661
   if (RISCVII::hasVecPolicyOp(MaskedMCID.TSFlags)) {
-    // The last operand of the pseudo is the policy op, but we might have a
-    // Glue operand last. We might also have a chain.
-    TailPolicyOpIdx = N->getNumOperands() - 1;
-    if (N->getOperand(*TailPolicyOpIdx).getValueType() == MVT::Glue)
-      (*TailPolicyOpIdx)--;
-    if (N->getOperand(*TailPolicyOpIdx).getValueType() == MVT::Other)
-      (*TailPolicyOpIdx)--;
-
+    TailPolicyOpIdx =
+        RISCVII::getVecPolicyOpNum(MaskedMCID) - MaskedMCID.getNumDefs();
----------------
This change is intended to be NFC right?  If so, please separate it for review.  As mentioned above, the correspondence of SDNode operands and MI operands is non-trivial.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130895