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

[asbirlea]

Summary

If MemorySSA is avaiable, we can skip checking all instructions if block has any Defs.
(volatile loads are also Defs).
We still need to check all instructions for "canThrow", even if no Defs are found.

[george.burgess.iv]

lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

852 ↗

(On Diff #183217)

Do atomic ops fall in the same "we care and should exit now" bucket as volatile loads for this?

MSSA uses MemoryDefs to represent fences and atomic loads with ordering > unordered.

[chandlerc]

LGTM (provided I've not missed anything below w.r.t. atomics).

lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

852 ↗

(On Diff #183217)

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.

[asbirlea]

Thank you for the review!

lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

852 ↗

(On Diff #183217)

The check below is mayHaveSideEffects, which internally calls mayWriteToMemory, which returns true for fences and unordered loads.
So, yes, we were already bailing for these.

llvm/trunk/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

static bool unswitchAllTrivialConditions(Loop &L, DominatorTree &DT,
   // conditions, this scenario could be very common in practice.
   BasicBlock *CurrentBB = L.getHeader();
   SmallPtrSet<BasicBlock *, 8> Visited;
   Visited.insert(CurrentBB);
   do {
     // Check if there are any side-effecting instructions (e.g. stores, calls,
     // volatile loads) in the part of the loop that the code *would* execute
     // without unswitching.
+    if (MSSAU) // Possible early exit with MSSA
+      if (auto *Defs = MSSAU->getMemorySSA()->getBlockDefs(CurrentBB))
+        if (!isa<MemoryPhi>(*Defs->begin()) || (++Defs->begin() != Defs->end()))
+          return Changed;
     if (llvm::any_of(*CurrentBB,
                      [](Instruction &I) { return I.mayHaveSideEffects(); }))
       return Changed;
 
     Instruction *CurrentTerm = CurrentBB->getTerminator();
 
     if (auto *SI = dyn_cast<SwitchInst>(CurrentTerm)) {
       // Don't bother trying to unswitch past a switch with a constant