What are the intended semantics of a volatile load when considered as a possible dependency?
Currently, we have a check for isVolatile with a nice comment about being overly conservative. However, this code turns out to be completely dead. Instead, this case is handled by the !isUnordered block above*. In the case I'm looking at (load PRE in GVN), we fail the queryInst test and return the same result as we would have in the isVolatile block, but there are cases where we allow volatile loads to be bypassed. (Specifically, when we can tell the queryInst isn't volatile.)
When I make it so that a volatile load is treated like a normal load, none of the tests fail. It also results in load PRE bypassing volatile loads which is good for the case I'm looking at, but the lack of any test failures disturbed me a bit.
I *think* it's perfectly legal to consider a volatile load as a normal load *unless* the query instruction we're testing against is itself volatile. Another way of phrasing this is that volatiles are only ordered with respects to other volatiles. Do others agree?
* For the sake of forthrightness, I was one of the (several) reviewers on change that added this and none of us noticed the possible issue.