SROA: Don't insert instructions before a PHI

[majnemer]

Summary

SROA may decide that it needs to insert a bitcast and would set it's
insertion point before a PHI. This will create an invalid module
right quick.

Instead, choose the first insertion point in the basic block that holds
our PHI.

The only testcase that I've got relies on a very specific uselist, I
haven't managed to come up with something nice and simple.

[chandlerc]

Needs a test case. Importantly, one triggering both the landing pad case and the PHI node case. =]

[majnemer]

• Address review comments.

[majnemer]

I don't think it's possible to hit the landingpad case because it can't coexist with a phi. I've added a testcase which reliably triggers the bug.

[chandlerc]

Minor tweaks to the test below, thanks!

test/Transforms/SROA/phi-and-select.ll

572 ↗

(On Diff #13137)

Need a CHECK-LABEL here.

I would also summarize the salient point of the test -- that we have a foldable PHI feeding a speculatable PHI which requires the rewriting of the speculated PHI to handle insertion when the incoming pointer is itself from a PHI node.

[majnemer]

Closed by commit rL216891 (authored by @majnemer).

lib/Transforms/Scalar/SROA.cpp

bool visitPHINode(PHINode &PN) {
     assert(BeginOffset >= NewAllocaBeginOffset && "PHIs are unsplittable");
     assert(EndOffset <= NewAllocaEndOffset && "PHIs are unsplittable");
 
     // We would like to compute a new pointer in only one place, but have it be
     // as local as possible to the PHI. To do that, we re-use the location of
     // the old pointer, which necessarily must be in the right position to
     // dominate the PHI.
     IRBuilderTy PtrBuilder(IRB);
+    if (isa<LandingPadInst>(OldPtr) || isa<PHINode>(OldPtr))
+      PtrBuilder.SetInsertPoint(OldPtr->getParent()->getFirstInsertionPt());
+    else
       PtrBuilder.SetInsertPoint(OldPtr);
     PtrBuilder.SetCurrentDebugLocation(OldPtr->getDebugLoc());
 
     Value *NewPtr = getNewAllocaSlicePtr(PtrBuilder, OldPtr->getType());
     // Replace the operands which were using the old pointer.
     std::replace(PN.op_begin(), PN.op_end(), cast<Value>(OldPtr), NewPtr);
 
     DEBUG(dbgs() << "          to: " << PN << "\n");
     deleteIfTriviallyDead(OldPtr);