[PATCH] D141256: [DAGCombine]Don't check for Undef/Poison if the node is deleted

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14356
+    return SDValue();
+  }
+  else{
----------------
RKSimon wrote:
> RKSimon wrote:
> > (style) early out instead of a if-else chain
> > ```
> > if(N0.getOpcode() == ISD::DELETED_NODE)
> >   return SDValue()
> > // NOTE: this strips poison generating flags.
> > SDValue R = DAG.getNode(N0.getOpcode(), SDLoc(N0), N0->getVTList(), Ops);
> > ```
> Can you confirm that the DELETED_NODE is appearing due to the UpdateNodeOperands call above (sorry you didn't generate the diff with context so I can't point to it)
> 
> I really need to step through the debugger myself, but can't right now :( 
OK - in which case this can be put straight after the UpdateNodeOperands:
```
  for (SDValue MaybePoisonOperand : MaybePoisonOperands) {
    // Don't replace every single UNDEF everywhere with frozen UNDEF, though.
    if (MaybePoisonOperand.getOpcode() == ISD::UNDEF)
      continue;
    // First, freeze each offending operand.
    SDValue FrozenMaybePoisonOperand = DAG.getFreeze(MaybePoisonOperand);
    // Then, change all other uses of unfrozen operand to use frozen operand.
    DAG.ReplaceAllUsesOfValueWith(MaybePoisonOperand, FrozenMaybePoisonOperand);
    if (FrozenMaybePoisonOperand.getOpcode() == ISD::FREEZE &&
        FrozenMaybePoisonOperand.getOperand(0) == FrozenMaybePoisonOperand) {
      // But, that also updated the use in the freeze we just created, thus
      // creating a cycle in a DAG. Let's undo that by mutating the freeze.
      DAG.UpdateNodeOperands(FrozenMaybePoisonOperand.getNode(),
                             MaybePoisonOperand);
    }
  }

  // Early-out if the node has already been updated in place.
  if (N0.getOpcode() == ISD::DELETED_NODE)
    return SDValue();

  // Finally, recreate the node, it's operands were updated to use
  // frozen operands, so we just need to use it's "original" operands.
  SmallVector<SDValue> Ops(N0->op_begin(), N0->op_end());
  // Special-handle ISD::UNDEF, each single one of them can be it's own thing.
  for (SDValue &Op : Ops) {
    if (Op.getOpcode() == ISD::UNDEF)
      Op = DAG.getFreeze(Op);
  }
  // NOTE: this strips poison generating flags.
  SDValue R = DAG.getNode(N0.getOpcode(), SDLoc(N0), N0->getVTList(), Ops);
  assert(DAG.isGuaranteedNotToBeUndefOrPoison(R, /*PoisonOnly*/ false) &&
         "Can't create node that may be undef/poison!");
  return R;
```


================
Comment at: llvm/test/CodeGen/AArch64/dagcombine-deleted-freeze.ll:4
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-linux-gnu"
+
----------------
drop the target/datalayout - and let the -mtriple in the RUN handle it


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

https://reviews.llvm.org/D141256