[llvm] 6c379cb - Scalarizer: fix an opaque pointer bug

[Nicolai Hähnle via llvm-commits]

Author: Nicolai Hähnle
Date: 2022-12-08T20:48:14+01:00
New Revision: 6c379cb318d74dfd63667dc62c026a5819f0b934

URL: https://github.com/llvm/llvm-project/commit/6c379cb318d74dfd63667dc62c026a5819f0b934
DIFF: https://github.com/llvm/llvm-project/commit/6c379cb318d74dfd63667dc62c026a5819f0b934.diff

LOG: Scalarizer: fix an opaque pointer bug

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

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

Modified: 
    llvm/lib/Transforms/Scalar/Scalarizer.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/Scalarizer.cpp b/llvm/lib/Transforms/Scalar/Scalarizer.cpp
index bbce942965ea..0517d2f32a20 100644
--- a/llvm/lib/Transforms/Scalar/Scalarizer.cpp
+++ b/llvm/lib/Transforms/Scalar/Scalarizer.cpp
@@ -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
index 000000000000..81737a184915
--- /dev/null
+++ b/llvm/test/Transforms/Scalarizer/opaque-ptr-bug.ll
@@ -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
+; 
diff erent 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
+}