[PATCH] D37460: [GVN] Prevent LoadPRE from hoisting through guards

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D37460#867397, @efriedma wrote:

> The volatile load thing never really got discussed in depth, as far as I remember... I wanted to be conservative because volatile memory access can be weird, but I guess my interpretation in isGuaranteedToTransferExecutionToSuccessor() is a little suspect given the way alias analysis treats volatile operations.


I think that it would be good to revisit this. As you say, we'd also need to model this in AA (by essentially making all volatile access universally aliasing because the trap handlers could be arbitrary functions). Moreover, it would essentially mean modeling volatile loads as both reading and writing to memory.

What I consider to be "regular" uses of volatile, handling memory-mapped I/O and communicating with signal/interrupt handlers, I don't think require this "loads also write" treatment. These use cases might want universally-aliasing stores (because the stores might trigger some device or signal/interrupt handler to do something arbitrary), but can allow more-precise AA for volatile loads.

I don't know exactly what use cases we want to support with volatile accesses. If it's essentially just memory-mapped I/O (for device drivers and the like), then it might make sense to make . If we other use cases, then it might not make sense (and, as Philip says, we might want some other mechanism).

If we do want to make volatile accesses universally aliasing and writing, then I think we should remove them from the IR all together and add some intrinsics instead for this use case.


https://reviews.llvm.org/D37460