[PATCH] D141891: [VP][DAGCombiner] Introduce generalized pattern match for vp sdnodes.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/ISDOpcodes.h:1348
+/// Translate this VP Opcode to its corresponding non-VP Opcode.
+std::optional<unsigned> getFunctionOpCodeForVP(unsigned Opcode,
+                                               bool hasFPExcept);
----------------
OpCode->Opcode to match the usual capitalization in SelectionDAG.


================
Comment at: llvm/include/llvm/CodeGen/ISDOpcodes.h:1352
+/// Translate this non-VP Opcode to its corresponding VP Opcode.
+unsigned getVPForFunctionOpCode(unsigned OpCode);
+
----------------
OpCode -> Opcode


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:846
+  bool match(SDValue OpN, unsigned OpCode) const {
+      return OpCode == OpN->getOpcode();
+  }
----------------
This is overindented


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:854
+  SDValue getNode(unsigned Opcode, const SDLoc &DL, EVT VT, SDValue Operand,
+                  Optional<SDNodeFlags> Flags = std::nullopt) {
+      if (Flags)
----------------
We don't usually use Optional for Flags. We just default it to SDNodeFlags()


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:865
+        return DAG.getNode(Opcode, DL, VT, N1, N2, *Flags);
+      else
+        return DAG.getNode(Opcode, DL, VT, N1, N2);
----------------
Drop else after return


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141891