[PATCH] D36581: [DAG] Fix Node Replacement in PromoteIntBinOp

[Nirav Dave via Phabricator via llvm-commits]

niravd created this revision.

When one operand is a user of another in a promoted binary operation
we may replace and delete the returned value before returning
triggering an assertion. Reorder node replacements to prevent this.

Fixes PR34137.


https://reviews.llvm.org/D36581

Files:
  lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  test/CodeGen/X86/pr34137.ll


Index: test/CodeGen/X86/pr34137.ll
===================================================================
--- /dev/null
+++ test/CodeGen/X86/pr34137.ll
@@ -0,0 +1,53 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
+
+ at var_3 = external global i16, align 2
+ at var_13 = external global i16, align 2
+ at var_212 = external global i64, align 8
+
+define void @pr34127() {
+; CHECK-LABEL: pr34127:
+; CHECK:       # BB#0: # %entry
+; CHECK-NEXT:    movzwl {{.*}}(%rip), %eax
+; CHECK-NEXT:    movw {{.*}}(%rip), %cx
+; CHECK-NEXT:    andw %ax, %cx
+; CHECK-NEXT:    andl %eax, %ecx
+; CHECK-NEXT:    movl %ecx, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:    xorl %edx, %edx
+; CHECK-NEXT:    testw %cx, %cx
+; CHECK-NEXT:    sete %dl
+; CHECK-NEXT:    andl %eax, %edx
+; CHECK-NEXT:    movq %rdx, {{.*}}(%rip)
+; CHECK-NEXT:    movw $0, (%rax)
+; CHECK-NEXT:    retq
+entry:
+  %a = alloca i32, align 4
+  %0 = load i16, i16* @var_3, align 2
+  %conv = zext i16 %0 to i32
+  %1 = load i16, i16* @var_3, align 2
+  %conv1 = zext i16 %1 to i32
+  %2 = load i16, i16* @var_13, align 2
+  %conv2 = zext i16 %2 to i32
+  %and = and i32 %conv1, %conv2
+  %and3 = and i32 %conv, %and
+  store i32 %and3, i32* %a, align 4
+  %3 = load i16, i16* @var_3, align 2
+  %conv4 = zext i16 %3 to i32
+  %4 = load i16, i16* @var_3, align 2
+  %conv5 = zext i16 %4 to i32
+  %5 = load i16, i16* @var_13, align 2
+  %conv6 = zext i16 %5 to i32
+  %and7 = and i32 %conv5, %conv6
+  %and8 = and i32 %conv4, %and7
+  %tobool = icmp ne i32 %and8, 0
+  %lnot = xor i1 %tobool, true
+  %conv9 = zext i1 %lnot to i32
+  %6 = load i16, i16* @var_3, align 2
+  %conv10 = zext i16 %6 to i32
+  %and11 = and i32 %conv9, %conv10
+  %conv12 = sext i32 %and11 to i64
+  store i64 %conv12, i64* @var_212, align 8
+  %conv14 = zext i1 undef to i16
+  store i16 %conv14, i16* undef, align 2
+  ret void
+}
Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -1167,22 +1167,23 @@
     SDValue RV =
         DAG.getNode(ISD::TRUNCATE, DL, VT, DAG.getNode(Opc, DL, PVT, NN0, NN1));
 
-    // New replace instances of N0 and N1
-    if (Replace0 && N0 && N0.getOpcode() != ISD::DELETED_NODE && NN0 &&
-        NN0.getOpcode() != ISD::DELETED_NODE) {
+    // We are always replacing N0/N1's use in N and only need
+    // additional replacements if there are additional uses.
+    Replace0 &= !N0->hasOneUse();
+    Replace1 &= (N0 != N1) && !N1->hasOneUse();
+
+    // Combine Op here so it is presreved past replacements.
+    CombineTo(Op.getNode(), RV);
+
+    if (Replace0) {
       AddToWorklist(NN0.getNode());
       ReplaceLoadWithPromotedLoad(N0.getNode(), NN0.getNode());
     }
-
-    if (Replace1 && N1 && N1.getOpcode() != ISD::DELETED_NODE && NN1 &&
-        NN1.getOpcode() != ISD::DELETED_NODE) {
+    if (Replace1) {
       AddToWorklist(NN1.getNode());
       ReplaceLoadWithPromotedLoad(N1.getNode(), NN1.getNode());
     }
-
-    // Deal with Op being deleted.
-    if (Op && Op.getOpcode() != ISD::DELETED_NODE)
-      return RV;
+    return Op;
   }
   return SDValue();
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36581.110581.patch
Type: text/x-patch
Size: 3290 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170810/2db84f95/attachment.bin>