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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:1204
+         BBI != SI.getIterator(); ++BBI)
+      if (AA->getModRefInfo(&*BBI, MemoryLocation::get(&SI)) == ModRefInfo::Mod)
+        return nullptr;
----------------
lebedev.ri wrote:
> 1. Is it bitfield? Should we be checking `if (AA->getModRefInfo(&*BBI, MemoryLocation::get(&SI) & ModRefInfo::Mod)` ?
> 2. This does correctly work for calls right, not just instructions?
This looks better, but i don't have prior expirience with `ModRefInfo`,
so this still looks suspicious to me - is this conservatively correct?
`isModSet()` looks for `MustMod`, which seems stronger than `Mod`?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:1201-1203
+    // Make sure memory isn't modified between the two.
+    for (BasicBlock::iterator BBI = Load->getIterator();
+         BBI != SI.getIterator(); ++BBI)
----------------
Hm, how do we know both the load and the store are in the same basic block?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71828