[PATCH] D145272: [RISCV] Teach performCombineVMergeAndVOps to combine unmasked TU vpmerge with a masked MU TA op.

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:3162
-// Try to fold VMERGE_VVM with unmasked intrinsic to masked intrinsic. The
-// peephole only deals with VMERGE_VVM which is TU and has false operand same as
-// its true operand now. E.g. (VMERGE_VVM_M1_TU False, False, (VADD_M1 ...),
----------------
I'm missing the point where the old code "only dealt with VMERGE_VVM which is TU", since the code appears to handle TA just fine.

I can't even see the restriction where "false operand same as its true operand".

We seem to be taking a lot of this comment over to the new code, so I'm wondering if we're accurately describing the behaviour of this peephole.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:3199
+    SDValue MergeOpTrue = True->getOperand(0);
+    // Both the merge op and the true op must have the same merge op. The merge
+    // op must have an all 1s mask since we're going to keep the mask from the
----------------
The nomenclature we're using is a little unclear, using "merge op" to mean two different things in the same sentence. I note you're saying "the vmerge instruction" above - maybe that's clearer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145272