[PATCH] D155071: [RISCV] Fold vmerge into its ops with smaller VL if known

[Luke Lau via Phabricator via llvm-commits]

luke added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:3333
+
+  // Because N and True must have the same merge operand, the "effective" body
+  // is the minimum of their VLs. For example, if we have VL=3 and VL=5:
----------------
reames wrote:
> Nit pick - have the same merge operand *or the merge on the True instruction doesn't exist and is thus undef*.
My mental model of this is that we need to have the same merge operand for both, but if the merge operand on True is undefined or missing, then we assume it's the same merge operand as N. Could we make this reasoning more explicit in the preceding code?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:3334
+  // Because N and True must have the same merge operand, the "effective" body
+  // is the minimum of their VLs. For example, if we have VL=3 and VL=5:
+  //
----------------
reames wrote:
> This comment is awfully verbose, and I don't think the example helps much.
After reading this the next day, I agree. I think I was using the comments as a mental scratch pad


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155071