[PATCH] D31726: AliasAnalysis: Be less conservative about volatile than atomic.

Daniel Berlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 5 17:33:09 PDT 2017


dberlin added inline comments.


================
Comment at: test/Transforms/Util/MemorySSA/volatile-clobber.ll:27
+; MemorySSA does not try to ensure ordering
+; only aliasing.
 ; CHECK-LABEL define void @volatile_only
----------------
chandlerc wrote:
> dberlin wrote:
> > Note: we could leave them (volatile loads) as defs if we wanted by overriding getModRefInfo, but this is also consistent with what we do for other cases.
> > 
> > (if we left them as defs, they would link together all volatile loads and stores, everything else would bypass them.
> > 
> > 
> I think volatile loads should be defs for other volatile loads, even if the two pointers are known to not alias. I'm not sure how to express this though.
Note:

MemorySSA does not pretend in any way to represent ordering constraints (it can't sanely do so as part of the default chain without losing optimization).
That said what you want is certainly possible, it just adds extra phis, extra defs, extra things that every other load must bypass or prove equal.

What you'd actually get would be stores + volatile loads as defs.

IE a volatile load would be a def for a regular store as well.

Calling getclobberingaccess on the store would give a correct answer, but costs something.

So we could do it as part of the default chain of course. Like i said, it just blocks optimization. It also tends to confuse things, as there is a real difference between "does this write or read the same memory as that"  and "can i reorder these two memory operations".

Memdep tries to return clobbers to let things handle the second case, but it misses cases.

So if your goal is to be able to find the next volatile load, from a volatile load, for reordering, we should just add that, but my inclination (without data) would *not* try to make them defs. Because they aren't defs.  They are uses. Just ordered ones :)

(Note, of course, atomics that same visible effect as writing memory are already defs and stay defs no matter what we do)

There are a couple ways to  go about representing the ordering constraint:

Add an orderedmemoryuse/def, use it for them, it has an extra operand representing the ordering chain (IE the next thing that it can't be move above)

This would require orderedmemoryphis for that chain, but honestly, 99% of the cost of memoryssa is the use optimization and aliasing, not the initial chain building.   Since the the ordering rules themselves are not anywhere as expensive as aliasing, ....

So i'd expect this to cost nearly nothing, but be completely precise for loads (We already know what the next thing that  *reads or writes that memory* is, and can combine with that to give precise answers).

The downside is you have two chains.  But that basically amounts to: If you use the default chain, you don't touch ordered operations (IE try to replace them).
If you use both chains at once, you get precise answers about both properties.

Of course, just a thought.
I haven't tried it in practice yet :)

(any happy to make volatiles defs against other volatiles in the meanwhile if we like)


https://reviews.llvm.org/D31726





More information about the llvm-commits mailing list