[PATCH] D98240: [VectorCombine] Simplify to scalar store if only one element updated

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Sorry for completely ignoring this :(
I'm fine with fixing this up as a followup, if that happens today.



================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:810-813
+    StoreInst *NSI = Builder.CreateStore(NewElement, GEP);
+    NSI->copyMetadata(*SI);
+    if (SI->getAlign() < NSI->getAlign())
+      NSI->setAlignment(SI->getAlign());
----------------
I'm certain this is a miscompile.
The alignment that was valid for a vector store
is not guaranteed to be valid for the store of a single vector element,
unless it's the 0'th element of course.

I think this needs to be something like
```
newalign = 1;
if(auto*C = dyn_cast<ConstantInt>(Idx)) {
  newalign = max(old store align, old load align);
  newalign = commonAlignment(newalign, Idx * DL.getTypeSize(NewElement.getType()))
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98240/new/

https://reviews.llvm.org/D98240