[PATCH] D147255: [InstCombine][OpaquePtr] Do not try to match store type to value type for opaque stores

[Nikita Popov via Phabricator via llvm-commits]

nikic requested changes to this revision.
nikic added a comment.
This revision now requires changes to proceed.

I don't think your characterization here is correct. The point of the transform is to remove the bitcast of the stored value. With typed pointers, this shifted the bitcast from the stored value to the pointer. With opaque pointers, it removes the bitcast entirely. As such, I don't really understand the argument for why we should stop making this transform with opaque pointers. If you look through the test diffs, this patch is causing many optimization regressions.

Besides, even for your motivating case, this is only going to shift the problem in the different direction: What if you had a bitcasted i16 store and a half store to start with, and this transform made sure the types are the same? It's trading one case for another.

> We should probably not require type matching on memory ops in these passes

This would be the correct way to address this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147255