[PATCH] D87149: [InstCombine] erase instructions leading up to unreachable

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> I will point out that of all the modern compilers I could test, only Clang trunk deletes volatile stores, even followed by clearly unreachable code: https://compiler-explorer.com/z/EGTofn

Other compilers make effectively equivalent assumptions, even if they don't actively delete volatile operations before an unreachable.  See https://compiler-explorer.com/z/7a81Gj : every non-clang compiler will hoist a divide by zero past a volatile operation.  So I'm not sure we can claim there's any uniformity here.

> One specific example from the above: I think that C and C++ volatile accesses need to be usable to reliably trigger a fault due to failed virtual address translation or bad permissions on the mapped page on Linux and similar virtual memory operating systems. This is, for example, heavily used by kernels and other systems level code. It is even used in userspace for fast bounds checking and in some cases deoptimization. On many systems (certainly Linux userspace) these accesses should reliably trigger a signal handler. Signal handlers are not, AFAIK, guaranteed to return.
>
> Given that, it seems very hard to argue that a volatile memory access must not trap, and must not unwind the stack -- signal handlers are allowed to do that, and it seems like a long held valid use case for volatile access to memory is to trigger a specific signal handler. Perhaps this use case just didn't come up in the prior discussions?

A lot of the things you might want to do in a signal handler are things code on another thread could do anyway.  But there's some design space here, I guess.

> Another use case I have is very specifically to make a store not dead to the compiler if it is reachable. Benchmark utilities have relied on this usage of volatile for many years, and it would seem painful to try to re-teach people to use a different facility.

This should be covered by the existing semantics.

-----

Assuming a volatile operation can terminate the thread, or maybe siglongjmp, probably doesn't hurt optimization too badly.  (A little, but isGuaranteedToTransferExecutionToSuccessor() isn't necessary for most important optimizations.)

The thing that's probably most important from an optimization perspective is that we don't want to treat volatile as a synchronization barrier (so it could modify arbitrary memory); that would seriously hurt optimizations around volatile operations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87149