[PATCH] D37424: [ForwardOp] Remove read accesses for all instructions that have been moved

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

I assume that not everyone subscribed to llvm-commits knows that "ForwardOpTree"/"ForwardOp" is a component of Polly. The tag "[Polly]" might still be useful.

In https://reviews.llvm.org/D37424#859999, @grosser wrote:

> This seems to work, but my commit message is not very good yet. Can you suggest a commit message that describes the reason you prefer this solution?


Before this patch, OpTree did not consider forwarding an operand tree consisting of only single LoadInst as useful. The motivation was that, like an access to a read-only variable, it would just replace one MemoryAccess by another. However, in contrast to read-only accesses, this would replace a scalar access by an array access, which is something worth doing.

In addition, leaving scalar MemoryAccess is problematic in that VirtualUse prioritizes inter-Stmt use over intra-Stmt. It was possible that the same LLVM value has a MemoryAccess for accessing the remote Stmt's LoadInst as well as having the same LoadInst in its own instruction list (due to being forwarded from another operand tree).

With this patch we ensure that if a LoadInst is forwarded is any operand tree, also the operand tree containing just the LoadInst is forwarded as well, which effectively removes the scalar MemoryAccess such that only the array access remains, not both.


https://reviews.llvm.org/D37424