[PATCH] D139359: Scalarizer: fix an opaque pointer bug

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle created this revision.
nhaehnle added reviewers: serge-sans-paille, bjope, lebedev.ri, foad.
Herald added a subscriber: hiraditya.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added a project: LLVM.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139359

Files:
  llvm/lib/Transforms/Scalar/Scalarizer.cpp
  llvm/test/Transforms/Scalarizer/opaque-ptr-bug.ll


Index: llvm/test/Transforms/Scalarizer/opaque-ptr-bug.ll
===================================================================
--- /dev/null
+++ 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
+; 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
+}
Index: llvm/lib/Transforms/Scalar/Scalarizer.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/Scalarizer.cpp
+++ llvm/lib/Transforms/Scalar/Scalarizer.cpp
@@ -76,10 +76,13 @@
 // 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 @@
     // 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 @@
     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 @@
 
   // 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];


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D139359.480204.patch
Type: text/x-patch
Size: 3687 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221205/bdc94d39/attachment.bin>