Scalarizer: fix an opaque pointer bug
authorNicolai Hähnle <nicolai.haehnle@amd.com>
Mon, 5 Dec 2022 17:21:59 +0000 (18:21 +0100)
committerNicolai Hähnle <nicolai.haehnle@amd.com>
Thu, 8 Dec 2022 19:48:14 +0000 (20:48 +0100)
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.

Differential Revision: https://reviews.llvm.org/D139359

llvm/lib/Transforms/Scalar/Scalarizer.cpp
llvm/test/Transforms/Scalarizer/opaque-ptr-bug.ll [new file with mode: 0644]

index bbce942..0517d2f 100644 (file)
@@ -76,10 +76,13 @@ BasicBlock::iterator skipPastPhiNodesAndDbg(BasicBlock::iterator 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.
@@ -389,7 +392,7 @@ Scatterer ScalarizerVisitor::scatter(Instruction *Point, Value *V,
     // 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
@@ -406,7 +409,7 @@ Scatterer ScalarizerVisitor::scatter(Instruction *Point, Value *V,
     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.
@@ -422,7 +425,7 @@ void ScalarizerVisitor::gather(Instruction *Op, const ValueVector &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];
diff --git a/llvm/test/Transforms/Scalarizer/opaque-ptr-bug.ll b/llvm/test/Transforms/Scalarizer/opaque-ptr-bug.ll
new file mode 100644 (file)
index 0000000..81737a1
--- /dev/null
@@ -0,0 +1,28 @@
+; 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
+}