Scalarizer: fix an opaque pointer bug

[nhaehnle]

Summary

With opaque pointers, it's possible for the same pointer value to be
used to store different vector types (both number and type of elements),
so we need to take that into account when caching the scattering.

[nikic]

LGTM

llvm/lib/Transforms/Scalar/Scalarizer.cpp

BasicBlock::iterator skipPastPhiNodesAndDbg(BasicBlock::iterator Itr) {
   if (Itr != BB->end())
     Itr = skipDebugIntrinsics(Itr);
   return Itr;
 }
 
 // Used to store the scattered form of a vector.
 using ValueVector = SmallVector<Value *, 8>;
 
-// Used to map a vector Value to its scattered form.  We use std::map
-// because we want iterators to persist across insertion and because the
-// values are relatively large.
-using ScatterMap = std::map<Value *, ValueVector>;
+// Used to map a vector Value and associated type to its scattered form.
+// The associated type is only non-null for pointer values that are "scattered"
+// when used as pointer operands to load or store.
+//
+// We use std::map because we want iterators to persist across insertion and
+// because the values are relatively large.
+using ScatterMap = std::map<std::pair<Value *, Type *>, ValueVector>;
 
 // Lists Instructions that have been replaced with scalar implementations,
 // along with a pointer to their scattered forms.
 using GatherList = SmallVector<std::pair<Instruction *, ValueVector *>, 16>;
 
 // Provides a very limited vector-like interface for lazily accessing one
 // component of a scattered vector or vector pointer.
 class Scatterer {

 // vector or a pointer to a vector.
 Scatterer ScalarizerVisitor::scatter(Instruction *Point, Value *V,
                                      Type *PtrElemTy) {
   if (Argument *VArg = dyn_cast<Argument>(V)) {
     // Put the scattered form of arguments in the entry block,
     // so that it can be used everywhere.
     Function *F = VArg->getParent();
     BasicBlock *BB = &F->getEntryBlock();
-    return Scatterer(BB, BB->begin(), V, PtrElemTy, &Scattered[V]);
+    return Scatterer(BB, BB->begin(), V, PtrElemTy, &Scattered[{V, PtrElemTy}]);
   }
   if (Instruction *VOp = dyn_cast<Instruction>(V)) {
     // When scalarizing PHI nodes we might try to examine/rewrite InsertElement
     // nodes in predecessors. If those predecessors are unreachable from entry,
     // then the IR in those blocks could have unexpected properties resulting in
     // infinite loops in Scatterer::operator[]. By simply treating values
     // originating from instructions in unreachable blocks as undef we do not
     // need to analyse them further.
     if (!DT->isReachableFromEntry(VOp->getParent()))
       return Scatterer(Point->getParent(), Point->getIterator(),
                        PoisonValue::get(V->getType()), PtrElemTy);
     // Put the scattered form of an instruction directly after the
     // instruction, skipping over PHI nodes and debug intrinsics.
     BasicBlock *BB = VOp->getParent();
     return Scatterer(
         BB, skipPastPhiNodesAndDbg(std::next(BasicBlock::iterator(VOp))), V,
-        PtrElemTy, &Scattered[V]);
+        PtrElemTy, &Scattered[{V, PtrElemTy}]);
   }
   // In the fallback case, just put the scattered before Point and
   // keep the result local to Point.
   return Scatterer(Point->getParent(), Point->getIterator(), V, PtrElemTy);
 }
 
 // Replace Op with the gathered form of the components in CV.  Defer the
 // deletion of Op and creation of the gathered form to the end of the pass,
 // so that we can avoid creating the gathered form if all uses of Op are
 // replaced with uses of CV.
 void ScalarizerVisitor::gather(Instruction *Op, const ValueVector &CV) {
   transferMetadataAndIRFlags(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];
+  ValueVector &SV = Scattered[{Op, nullptr}];
   if (!SV.empty()) {
     for (unsigned I = 0, E = SV.size(); I != E; ++I) {
       Value *V = SV[I];
       if (V == nullptr || SV[I] == CV[I])
         continue;
 
       Instruction *Old = cast<Instruction>(V);
       if (isa<Instruction>(CV[I]))

llvm/test/Transforms/Scalarizer/opaque-ptr-bug.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt %s -passes='scalarizer,dce' -S -scalarize-load-store -o - | FileCheck %s
+
+; This used to crash because the same (pointer) value was scattered by
+; different amounts.
+
+define void @test1(ptr %p) {
+; CHECK-LABEL: @test1(
+; CHECK-NEXT:    [[P_I12:%.*]] = getelementptr i16, ptr [[P:%.*]], i32 1
+; CHECK-NEXT:    [[P_I11:%.*]] = getelementptr i32, ptr [[P]], i32 1
+; CHECK-NEXT:    [[P_I2:%.*]] = getelementptr i32, ptr [[P]], i32 2
+; CHECK-NEXT:    [[P_I3:%.*]] = getelementptr i32, ptr [[P]], i32 3
+; CHECK-NEXT:    store i32 0, ptr [[P]], align 8
+; CHECK-NEXT:    [[P_I1:%.*]] = getelementptr i32, ptr [[P]], i32 1
+; CHECK-NEXT:    store i32 0, ptr [[P_I1]], align 4
+; CHECK-NEXT:    store i32 0, ptr [[P]], align 16
+; CHECK-NEXT:    store i32 0, ptr [[P_I11]], align 4
+; CHECK-NEXT:    store i32 0, ptr [[P_I2]], align 8
+; CHECK-NEXT:    store i32 0, ptr [[P_I3]], align 4
+; CHECK-NEXT:    store i16 0, ptr [[P]], align 4
+; CHECK-NEXT:    store i16 0, ptr [[P_I12]], align 2
+; CHECK-NEXT:    ret void
+;
+  store <2 x i32> zeroinitializer, ptr %p
+  store <4 x i32> zeroinitializer, ptr %p
+  store <2 x i16> zeroinitializer, ptr %p
+  ret void
+}