[PATCH] D71828: [InstCombine] Convert vector store to scalar store if only one element updated

Tue Jun 9 09:19:43 PDT 2020

qiucf marked an inline comment as done.
qiucf added a comment.

Thanks for comments!

In D71828#2082229 <https://reviews.llvm.org/D71828#2082229>, @spatel wrote:

> We have -vector-combine now, and it only runs late in the pipeline. If we put this transform in there, that should bypass concerns about interfering with GVN. That pass also has access to the cost model, so we could try to limit the transform if there are known cases where it would not be profitable (ie, no need to try to implement a codegen reversal).

I'll try moving this into vector combine. Maybe we can do/know more by help from TTI.

================
Comment at: llvm/test/Transforms/InstCombine/load-insert-store.ll:2
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S -instcombine < %s | FileCheck %s

----------------
lebedev.ri wrote:
> I wasn't sure whether or not this is endian-sensitive, alive2 says it's not, but maybe we should be proactive and do
> ```
> ; RUN: opt < %s -instcombine -S -data-layout=e | FileCheck %s --check-prefix=ALL --check-prefixes=CHECK,LE
> ; RUN: opt < %s -instcombine -S -data-layout=E | FileCheck %s --check-prefix=ALL --check-prefixes=CHECK,BE
> ```
> 
We have few docs explicitly talked about this, and I remember endianness doesn't matter. But adding BE/LE check is fine. Thanks.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71828