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

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added inline comments.


================
Comment at: lib/Analysis/AliasAnalysis.cpp:336
+  // Be conservative in the face of atomic.
+  if (isStrongerThan(L->getOrdering(), AtomicOrdering::NotAtomic))
     return MRI_ModRef;
----------------
dberlin wrote:
> dberlin wrote:
> > sanjoy wrote:
> > > Why not `isStrongerThan(L->getOrdering(), AtomicOrdering::Unordered)`? (same below)
> > > 
> > > OTOH I'm surprised that this did not break any tests.
> > Honestly:
> > Because i don't know enough to know if that's okay ;)
> > If it is, happy to do it.
> > 
> > It breaks 1 optimizations test that i'm looking at:
> > 
> > memcpy/atomic.ll optimizes *less* now.
> > Which makes no sense at all.  I suspect it was relying on weird aliasing before. I'm debugging.
> > 
> > Otherwise, it does break some memoryssa tests, which i'm updating for the correct answers.
> > (i'm also still awaiting 3 stage results on linux)
> > 
> Actually the memcpyopt is exactly the issue you point out, so fixing as part of test updates.
I think it is, but (more importantly) wasn't that the behavior before?

Previously if `L` was non-volatile AND (NotAtomic or Unordered) then we'd go through the normal alias analysis codepath.

Now you want to remove the "non-volatile AND" bit.  This would mean we'd want to go through the normal path if the load was NotAtomic or Unordered.


https://reviews.llvm.org/D31726