[PATCH] D93274: [DAGCombiner] Don't create sexts of deleted xors when they were in-visit replaced

[Layton Kifer via Phabricator via llvm-commits]

laytonio created this revision.
laytonio added reviewers: spatel, RKSimon, craig.topper, lebedev.ri, efriedma, nemanjai.
Herald added subscribers: ecnelises, steven.zhang, hiraditya.
laytonio requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Fixes a bug introduced by D91589 <https://reviews.llvm.org/D91589>.

When folding `(sext (not i1 x)) -> (add (zext i1 x), -1)`, we try to replace the not first when possible. If we replace the not in-visit, then the now invalidated node will be returned, and subsequently we will return an invalid sext. In cases where the not is replaced in-visit we can simply return SDValue, as the not in the current sext should have already been replaced.

Thanks @jgorbe, for finding the below reproducer.

The following reduced test case crashes clang when built with `clang -O1 -frounding-math`:

  template <class> class a {
    int b() { return c == 0.0 ? 0 : -1; }
    int c;
  };
  template class a<long>;

A debug build of clang produces this "assertion failed" error:

  clang: /home/jgorbe/code/llvm/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:264: void {anonymous}::DAGCombiner::AddToWorklist(llvm::
  SDNode*): Assertion `N->getOpcode() != ISD::DELETED_NODE && "Deleted Node added to Worklist"' failed.




Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93274

Files:
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp


Index: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -10730,8 +10730,18 @@
       (!LegalOperations || (TLI.isOperationLegal(ISD::ZERO_EXTEND, VT) &&
                             TLI.isOperationLegal(ISD::ADD, VT)))) {
     // If we can eliminate the 'not', the sext form should be better
-    if (SDValue NewXor = visitXOR(N0.getNode()))
-      return DAG.getNode(ISD::SIGN_EXTEND, DL, VT, NewXor);
+    if (SDValue NewXor = visitXOR(N0.getNode())) {
+      // Returning N0 is a form of in-visit replacement that may have
+      // invalidated N0.
+      if (NewXor.getNode() == N0.getNode()) {
+        // Return SDValue here as the xor should have already been replaced in
+        // this sext.
+        return SDValue();
+      } else {
+        // Return a new sext with the new xor.
+        return DAG.getNode(ISD::SIGN_EXTEND, DL, VT, NewXor);
+      }
+    }
 
     SDValue Zext = DAG.getNode(ISD::ZERO_EXTEND, DL, VT, N0.getOperand(0));
     return DAG.getNode(ISD::ADD, DL, VT, Zext, DAG.getAllOnesConstant(DL, VT));


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D93274.311784.patch
Type: text/x-patch
Size: 1200 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201215/b291ffe8/attachment.bin>