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

[Qiu Chaofan via Phabricator via llvm-commits]

qiucf added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:784
+  StoreInst *SI = dyn_cast<StoreInst>(&I);
+  if (!SI || !SI->isSimple() || !SI->getValueOperand()->getType()->isVectorTy())
+    return false;
----------------
fhahn wrote:
> I'm not sure if the vector GEP will properly handle types that are non-power-of-2 properly. Perhaps it might be good to limit this to types for which the following holds? `SI->typeSizeEqualsStoreSize(LI->getType())`?
Hmm.. Not sure what you mean, maybe something like

```
%0 = getelementptr inbounds <16 x i24>, <16 x i24>* %q, i32 0, i32 3
store i24 %s, i24* %0, align 4
```

this should be expected result. Or do we need to ensure scalar type size of load equals to store's value type size?


================
Comment at: llvm/test/Transforms/InstCombine/load-insert-store.ll:1
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -S -instcombine < %s | FileCheck %s
----------------
fhahn wrote:
> I'm not sure why this has been removed?
It's moved from `InstCombine` to `VectorCombine`, not deleted.


================
Comment at: llvm/test/Transforms/VectorCombine/X86/load-insert-store.ll:15
+  %vecins = insertelement <16 x i8> %0, i8 %s, i32 3
+  store <16 x i8> %vecins, <16 x i8>* %q
+  ret void
----------------
fhahn wrote:
> can you also add a test with element types > i8?
Do you mean `insertelement <16 x i8> %0, i32 %s, i32 2`? If not, there's a test for `<8 x i16>` below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98240