[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/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:854
+  SDValue getNode(unsigned Opcode, const SDLoc &DL, EVT VT, SDValue Operand,
+                  Optional<SDNodeFlags> Flags = std::nullopt) {
+      if (Flags)
----------------
fakepaper56 wrote:
> craig.topper wrote:
> > fakepaper56 wrote:
> > > craig.topper wrote:
> > > > We don't usually use Optional for Flags. We just default it to SDNodeFlags()
> > > I think using Optional is to represent both DAG.getCode() w/ SDNodeFlags and DAG.getCode()  w/o SDNodeFlags.
> > The SelectionDAG::getNode functions being called always take an SDNodeFlags they just default construct one if it isn't passed explicitly. Why can't we do the same in the context?
> I think  `SelectionDAG::getNode` uses `FlagInserter` when not having `SDNodeFlags` argument. The below code is the reason why I think so,
> ```
> SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
>                               SDValue Operand) {
>   SDNodeFlags Flags;
>   if (Inserter)
>     Flags = Inserter->getFlags();
>   return getNode(Opcode, DL, VT, Operand, Flags);
> }
> ```
Ok. I forgot about flag inserter. We used to default construct flags. I have another idea I'm going to try


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141891