Page MenuHomePhabricator

[AArch64][GlobalISel] Handle multiple phis in fixupPHIOpBanks

Authored by paquette on Jun 2 2021, 5:52 PM.



If we ended up with two phi instructions in a block, and we needed to fix up the banks for the first one, we'd end up inserting our COPY before the second phi.


%x = G_PHI ...
%fixup = COPY ...
%y = G_PHI ...

This is invalid MIR, and breaks assumptions made by the register allocator later down the line. With the verifier enabled, it also emits a verification error.

This teaches fixupPHIOpBanks to walk past any phi instructions in the block when emitting the fixup copies.

Here's an example of the crashing code (same as added testcase):

Diff Detail

Event Timeline

paquette created this revision.Jun 2 2021, 5:52 PM
paquette requested review of this revision.Jun 2 2021, 5:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2021, 5:53 PM
aemerson added inline comments.Jun 3 2021, 10:21 AM

Can we simply use getFirstNonPHI() unconditionally?

paquette added inline comments.Jun 3 2021, 2:31 PM

No, because the point where we're inserting the COPY may be in a block without phis.

e.g. in test_loop_phi_gpr_to_fpr:

  successors: %bb.2(0x80000000)

  %6:gpr(s32) = G_IMPLICIT_DEF
  %7:gpr(s32) = G_SELECT %0(s1), %6, %6
  %1:gpr(s16) = G_TRUNC %7(s32)


In this case, OpDef is the G_TRUNC. So, we need to insert the copy after it (at the end of the block). So, inserting after the first non-phi instruction isn't correct here.

aemerson accepted this revision.Jun 4 2021, 9:21 AM


This revision is now accepted and ready to land.Jun 4 2021, 9:21 AM