This is an archive of the discontinued LLVM Phabricator instance.

Refine memory dependence's notion of volatile semantics
ClosedPublic

Authored by reames on Jan 9 2015, 11:19 AM.

Details

Summary

(For reviewers who get duplicates, sorry! Forgot to add llvm-commits to the first review and had to close it and open another.)

According to my reading of the LangRef, volatiles are only ordered with respect to other volatiles. It is entirely legal and profitable to forward unrelated loads over the volatile load. This patch implements this for GVN by refining the transition rules MemoryDependenceAnalysis uses when encountering a volatile.

The added test cases show where the extra flexibility is profitable for local dependence optimizations. I have a related patch (http://reviews.llvm.org/D6895) which will extend this to non-local dependence (i.e. PRE), but that's essentially orthogonal to the semantic change in this patch. I have tested the two together and can confirm that PRE works over a volatile load with both changes.

One question for reviewers: should a load being volatile change the default handling for a *may* alias load? For a non-volatile load, we'll keep scanning past looking for an actual mustalias dependency. Is this the behaviour we want for volatiles? While I think this matches the letter of the LangRef, I'm not entirely sure it matches the expectations of mapping C volatile to LLVM volatile. In particular, should the following code always return 0? With my current patch, we can and will forward the first load to third without changing the second.
int example(int* p, int *q) { // q == p at runtime

int x = *p;
*(volatile int*)q
int y = *p;
return x-y;

}

Diff Detail

Event Timeline

reames updated this revision to Diff 17937.Jan 9 2015, 11:19 AM
reames retitled this revision from to Refine memory dependence's notion of volatile semantics.
reames updated this object.
reames edited the test plan for this revision. (Show Details)
reames added a subscriber: Unknown Object (MLST).
hfinkel accepted this revision.Jan 23 2015, 6:14 PM
hfinkel edited edge metadata.

LGTM.

One question for reviewers: should a load being volatile change the default handling for a *may* alias load?

No, it should not.

FWIW, LLVM's volatile is even more strict than C's volatile. LLVM's volatile is meaningful on loads, and is specified as:

The optimizers must not change the number of volatile operations or change their order of execution relative to other volatile operations. The optimizers may change the order of volatile operations relative to non-volatile operations.

Whereas C's volatile says (5.1.2.3):

An actual implementation need not evaluate part of an expression if it can deduce that its value is not used and that no needed side effects are produced (including any caused by calling a function or accessing a volatile object).

Which is made even more explicit under the specification of assignment semantics 6.5.16:

An assignment operator stores a value in the object designated by the left operand. An assignment expression has the value of the left operand after the assignment (The implementation is permitted to read the object to determine the value but is not required to, even when the object has volatile-qualified type.)

and, thus, is not really meaningful on loads.

This revision is now accepted and ready to land.Jan 23 2015, 6:14 PM
This revision was automatically updated to reflect the committed changes.