[Scalarizer] PR28108: Skip over nullptr rather than crashing on it.

[uabelho]

Summary

In Scalarizer::gather we see if we already have a scattered form of Op,
and in that case use the new form.

In the particular case of PR28108, the found ValueVector SV has size 2,
where the first Value is nullptr, and the second is indeed a proper Value.
The nullptr then caused an assert to blow when we tried to do
cast<Instruction>(SV[I]).

With this patch we check SV[I] before doing the cast, and if it's nullptr
we just skip over it.

I don't know the Scalarizer well enough to know if this is the best fix
or if something should be done else where to prevent the nullptr from
being in the ValueVector at all, but at least this avoids the crash
and looking at the test case output it looks reasonable.

[uabelho]

I don't know who could review this so I added some people that seems to have done or reviewed things in the Scalarizer before.

[uabelho]

Had wrong name on the test case file.

[uabelho]

Ping. Anyone wanna review this?

[mehdi_amini]

LGTM.

[uabelho]

Thank you Mehdi.

I don't have commit rights, could you or someone else possibly commit this or is there something else I should do?

llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp

void Scalarizer::gather(Instruction *Op, const ValueVector &CV) {
 
   transferMetadata(Op, CV);
 
   // If we already have a scattered form of Op (created from ExtractElements
   // of Op itself), replace them with the new form.
   ValueVector &SV = Scattered[Op];
   if (!SV.empty()) {
     for (unsigned I = 0, E = SV.size(); I != E; ++I) {
-      Instruction *Old = cast<Instruction>(SV[I]);
+      Value *V = SV[I];
+      if (V == nullptr)
+        continue;
+
+      Instruction *Old = cast<Instruction>(V);
       CV[I]->takeName(Old);
       Old->replaceAllUsesWith(CV[I]);
       Old->eraseFromParent();
     }
   }
   SV = CV;
   Gathered.push_back(GatherList::value_type(Op, &SV));
 }

llvm/trunk/test/Transforms/Scalarizer/crash-bug.ll

+; RUN: opt %s -scalarizer -S -o - | FileCheck %s
+
+; Don't crash
+
+define void @foo() {
+  br label %bb1
+
+bb2:                                        ; preds = %bb1
+  %bb2_vec = shufflevector <2 x i16> <i16 0, i16 10000>,
+                           <2 x i16> %bb1_vec,
+                           <2 x i32> <i32 0, i32 3>
+  br label %bb1
+
+bb1:                                        ; preds = %bb2, %0
+  %bb1_vec = phi <2 x i16> [ <i16 100, i16 200>, %0 ], [ %bb2_vec, %bb2 ]
+;CHECK: bb1:
+;CHECK: %bb1_vec.i0 = phi i16 [ 100, %0 ], [ 0, %bb2 ]
+;CHECK: %bb1_vec.i1 = phi i16 [ 200, %0 ], [ %bb1_vec.i1, %bb2 ]
+  br i1 undef, label %bb3, label %bb2
+
+bb3:
+  ret void
+}
+