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: _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
