[SDAG] Fix Stale SDNode usage in visitAND

[niravd]

Summary

Reorder CombineTo Calls to prevent potential use of deleted node.
Fixes PR32372.

[RKSimon]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

3538–3542

Duplicate SDValue(Load, 0) - change to:

SDValue NewLoad(Load, 0);
CombineTo(N, NewLoad);

??

test/CodeGen/SystemZ/pr32372.ll

5

Rename to pr32372 ? Any further cleanup possible?

[niravd]

Minor cleanup

[niravd]

test/CodeGen/SystemZ/pr32372.ll

5

Spent a little time attempting to strip it down while preserving the bug and was unsuccessful.

[RKSimon]

LGTM @jnspaulsson do you have any comments?

[uweigand]

Test case LGTM. Thanks!

[niravd]

I think that's quorum. Will someone accept the revision?

[jonpa]

Thanks very much for fixing this - it looks like all parts are approved now.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

if ((N0.getOpcode() == ISD::EXTRACT_VECTOR_ELT &&
     case ISD::EXTLOAD: B = CanZextLoadProfitably; break;
     case ISD::ZEXTLOAD:
     case ISD::NON_EXTLOAD: B = true; break;
     }
 
     if (B && Constant.isAllOnesValue()) {
       // If the load type was an EXTLOAD, convert to ZEXTLOAD in order to
       // preserve semantics once we get rid of the AND.
       SDValue NewLoad(Load, 0);
+
+      // Fold the AND away. NewLoad may get replaced immediately.
+      CombineTo(N, NewLoad);
+
       if (Load->getExtensionType() == ISD::EXTLOAD) {
         NewLoad = DAG.getLoad(Load->getAddressingMode(), ISD::ZEXTLOAD,
                               Load->getValueType(0), SDLoc(Load),
                               Load->getChain(), Load->getBasePtr(),
                               Load->getOffset(), Load->getMemoryVT(),
                               Load->getMemOperand());
         // Replace uses of the EXTLOAD with the new ZEXTLOAD.
         if (Load->getNumValues() == 3) {
           // PRE/POST_INC loads have 3 values.
           SDValue To[] = { NewLoad.getValue(0), NewLoad.getValue(1),
                            NewLoad.getValue(2) };
           CombineTo(Load, To, 3, true);
         } else {
           CombineTo(Load, NewLoad.getValue(0), NewLoad.getValue(1));
         }
       }
 
-      // Fold the AND away, taking care not to fold to the old load node if we
-      // replaced it.
-      CombineTo(N, (N0.getNode() == Load) ? NewLoad : N0);
-
       return SDValue(N, 0); // Return N so it doesn't get rechecked!
     }
   }
 
   // fold (and (load x), 255) -> (zextload x, i8)
   // fold (and (extload x, i16), 255) -> (zextload x, i8)
   // fold (and (any_ext (extload x, i16)), 255) -> (zextload x, i8)
   if (!VT.isVector() && N1C && (N0.getOpcode() == ISD::LOAD ||

test/CodeGen/SystemZ/pr32372.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc %s -o - -mtriple=s390x-linux-gnu | FileCheck %s
+
+define void @pr32372(i8*) {
+; CHECK-LABEL: pr32372:
+; CHECK:       # BB#0: # %BB
+; CHECK-NEXT:    llc %r1, 0(%r2)
+; CHECK-NEXT:    mvhhi 0(%r1), -3825
+; CHECK-NEXT:    llill %r0, 0
+; CHECK-NEXT:    dlr %r0, %r1
+; CHECK-NEXT:  .LBB0_1: # %CF251
+; CHECK-NEXT:    # =>This Inner Loop Header: Depth=1
+; CHECK-NEXT:    j .LBB0_1
+BB:
+  %L = load i8, i8* %0
+  store i16 -3825, i16* undef
+  %L5 = load i8, i8* %0
+  %B9 = urem i8 %L5, %L
+  %I107 = insertelement <8 x i8> zeroinitializer, i8 %B9, i32 7
+  %ZE141 = zext i8 %L5 to i16
+  br label %CF251
+
+CF251:                                            ; preds = %CF258, %CF251, %BB
+  %Shuff217 = shufflevector <8 x i8> zeroinitializer, <8 x i8> %I107, <8 x i32> <i32 0, i32 2, i32 undef, i32 6, i32 8, i32 undef, i32 12, i32 14>
+  %Cmp227 = icmp sge i16 %ZE141, 0
+  br i1 %Cmp227, label %CF251, label %CF258
+
+CF258:                                            ; preds = %CF251
+  %Shuff230 = shufflevector <2 x i16> undef, <2 x i16> undef, <2 x i32> <i32 3, i32 1>
+  br label %CF251
+}