[PATCH] D150900: [InstCombine] Insert a bitcast to enable merging similar store insts

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D150900#4359334 <https://reviews.llvm.org/D150900#4359334>, @gandhi21299 wrote:

> The reason for another iteration of instcombine for @inttoptr_merge(..) is that mergeStoreIntoSuccessor() generates a case where a store/load pair is merged into an inttoptr which is later merged with the PHI inserted by mergeStoreIntoSuccessor() in the first iteration. I am not sure if implementing these cases in mergeStoreIntoSuccessor() is viable since it will make the code redundant.

I've looked into this as well, and I think this would mostly be solved by D75362 <https://reviews.llvm.org/D75362> (we visit instructions in the wrong order, in a way that is relevant here). It's okay to ignore this.

It took me a while to understand why AMDGPU is relevant here. The difference is that with default data layout i64 only has 4 byte alignment, so we first need to promote the alignment to 8 before the transform can be performed. For AMDGPU the alignment is 8 to start with. You can make this test AMDGPU independent by just explicitly specifying the alignment on the stores, instead of using natural alignment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150900