[PATCH] D57129: [SimpleLoopUnswitch] Early check exit for trivial unswitch with MemorySSA.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea marked 3 inline comments as done.
asbirlea added a comment.

Thank you for the review!



================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:852
+      if (auto *Defs = MSSAU->getMemorySSA()->getBlockDefs(CurrentBB))
+        if (!isa<MemoryPhi>(*Defs->begin()) || (++Defs->begin() != Defs->end()))
+          return Changed;
----------------
chandlerc wrote:
> george.burgess.iv wrote:
> > Do atomic ops fall in the same "we care and should exit now" bucket as volatile loads for this?
> > 
> > MSSA uses MemoryDefs to represent `fence`s and atomic loads with ordering > `unordered`.
> I think so yes, and so I think this code is correct?
> 
> Such loads "appear" to write to memory and thus I think should bail. At least, this code seems to be remain a good early exit. We still directly test every instruction below.
The check below is `mayHaveSideEffects`, which internally calls `mayWriteToMemory`, which returns `true` for `fence`s and `unordered` loads.
So, yes, we were already bailing for these. 



Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57129/new/

https://reviews.llvm.org/D57129