[llvm-branch-commits] [llvm] d29f93b - [DAGCombiner] Don't create sexts of deleted xors when they were in-visit replaced

[Jordan Rupprecht via llvm-branch-commits]

Author: Layton Kifer
Date: 2020-12-23T16:16:26-08:00
New Revision: d29f93bda5114aec596c0cbb1e3ce37b61c6398c

URL: https://github.com/llvm/llvm-project/commit/d29f93bda5114aec596c0cbb1e3ce37b61c6398c
DIFF: https://github.com/llvm/llvm-project/commit/d29f93bda5114aec596c0cbb1e3ce37b61c6398c.diff

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

Fixes a bug introduced by 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.
```

Reviewed By: spatel

Differential Revision: https://reviews.llvm.org/D93274

Added: 
    

Modified: 
    llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
    llvm/test/CodeGen/SystemZ/sext-zext.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 74d3e1adcd6c..92b23df9e3af 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -10721,8 +10721,18 @@ SDValue DAGCombiner::visitSIGN_EXTEND(SDNode *N) {
       (!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));

diff  --git a/llvm/test/CodeGen/SystemZ/sext-zext.ll b/llvm/test/CodeGen/SystemZ/sext-zext.ll
index d48e4ba83588..fbe1c44c1e50 100644
--- a/llvm/test/CodeGen/SystemZ/sext-zext.ll
+++ b/llvm/test/CodeGen/SystemZ/sext-zext.ll
@@ -28,6 +28,25 @@ define i32 @sext_of_not_cmp(i32 %x) {
   ret i32 %sext
 }
 
+;; fold (sext (not (setcc a, b, cc))) -> (sext (setcc a, b, !cc))
+;; make sure we don't crash if the not gets replaced in-visit
+define i32 @sext_of_not_fsetccs(double %x) {
+; CHECK-LABEL: sext_of_not_fsetccs:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    ltdbr %f0, %f0
+; CHECK-NEXT:    ipm %r0
+; CHECK-NEXT:    afi %r0, 1879048192
+; CHECK-NEXT:    srl %r0, 31
+; CHECK-NEXT:    lcr %r2, %r0
+; CHECK-NEXT:    br %r14
+  %cmp = call i1 @llvm.experimental.constrained.fcmp.f64(double %x, double 0.000000e+00, metadata !"oeq", metadata !"fpexcept.ignore")
+  %xor = xor i1 %cmp, 1
+  %sext = sext i1 %xor to i32
+  ret i32 %sext
+}
+
+declare i1 @llvm.experimental.constrained.fcmp.f64(double, double, metadata, metadata)
+
 ;; TODO: fold (add (zext (setcc a, b, cc)), -1) -> (sext (setcc a, b, !cc))
 define i32 @dec_of_zexted_cmp(i32 %x) {
 ; CHECK-LABEL: dec_of_zexted_cmp: