[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 15:29:09 PDT 2017
dberlin marked 4 inline comments as done.
dberlin 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;
----------------
sanjoy wrote:
> 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.
Yes, you are correct., fixed.
https://reviews.llvm.org/D31726
More information about the llvm-commits
mailing list