This is an archive of the discontinued LLVM Phabricator instance.

LiveIntervalAnalysis: Fix alias regunit reserved definition
ClosedPublic

Authored by MatzeB on Aug 31 2017, 2:13 PM.

Details

Summary

A register in CodeGen can be marked as reserved: In that case we
consider the register always live and do not use (or rather ignore)
kill/dead/undef operand flags.

LiveIntervalAnalysis however tracks liveness per register unit (not per
register). We already needed adjustments for this in r292871 to deal
with super/sub registers. However I did not look at aliased register
there. Looking at ARM:

FPSCR (regunits FPSCR, FPSCR~FPSCR_NZCV) aliases with FPSCR_NZCV
(regunits FPSCR_NZCV, FPSCR~FPSCR_NZCV) hence they share a register unit
(FPSCR~FPSCR_NZCV) that represents the aliased parts of the registers.
This shared register unit was previously considered non-reserved,
however given that we uses of the reserved FPSCR potentially violate
some rules (like uses without defs) we should make FPSCR~FPSCR_NZCV
reserved too and stop tracking liveness for it.

This patch:

  • Defines a register unit as reserved when: At least for one root register, the root register and all its super registers are reserved.
  • Adjust LiveIntervals::computeRegUnitRange() for new reserved definition.
  • Add MachineRegisterInfo::isReservedRegUnit() to have a canonical way of testing.
  • Stop computing LiveRanges for reserved register units in HMEditor even with UpdateFlags enabled.
  • Skip verification of uses of reserved reg units in the machine verifier (this usually didn't happen because there would be no cached liverange but there is no guarantee for that and I would run into this case before the HMEditor tweak, so may as well fix the verifier too).

Note that this should only affect ARMs FPSCR/FPSCR_NZCV registers today;
aliased registers are rarely used, the only other cases are hexagons
P0-P3/P3_0 and C8/USR pairs which are not mixing reserved/non-reserved
registers in an alias.

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB created this revision.Aug 31 2017, 2:13 PM
kparzysz added inline comments.Aug 31 2017, 2:28 PM
lib/CodeGen/LiveIntervalAnalysis.cpp
279

Should this still happen if Reg is reserved? For reserved units, the LR will only have dead defs. Is that the intent?

MatzeB added inline comments.Aug 31 2017, 2:49 PM
lib/CodeGen/LiveIntervalAnalysis.cpp
279

The code in RegisterCoalescer::joinReservedPhysReg() expects it that way. The comment there reads:

// The live range of the reserved register will look like a set of dead defs
// - we don't properly track the live range of reserved registers.
kparzysz accepted this revision.Sep 1 2017, 10:38 AM

On a somewhat unrelated note---is it valid on AArch64 to reserve W1_W2 without reserving W1 and W2? If so, it would lead to a situation where a reserved register does not contain any reserved units. If not, it would require one or both of W0_W1 and W2_W3 to also be reserved.

This revision is now accepted and ready to land.Sep 1 2017, 10:38 AM
MatzeB added a comment.EditedSep 1 2017, 11:11 AM

Thanks for the review!

On a somewhat unrelated note---is it valid on AArch64 to reserve W1_W2 without reserving W1 and W2? If so, it would lead to a situation where a reserved register does not contain any reserved units. If not, it would require one or both of W0_W1 and W2_W3 to also be reserved.

Yes this is allowed. That is exactly the situation I worked on in r292871. A typical situation you see in GPU targets is one or two registers reserved as stackpointer or similar ABI purposes, requiring that we also reserve all the tuples involving that register (usually done with markSuperRegs() in the targets getReservedRegs()). However just because one register in the tuple is reserved doesn't necessarily mean the other registers (when seen independently of the tuple) are reserved. Hence the logic in here that only considers a register unit as reserved if the register unit root register and all the super registers are reserved. This is for the situation of super/sub registers!

It just turned out that for aliased registers (the only situation in which we have more than 1 register unit root) requiring all register unit roots to be reserved, in order for the register unit to be reserved wasn't too helpful in ARMs case, where the reserved register was sometimes used in ways that would be invalid for a non-reserved register. So we wouldn't be able to compute liveness for the special register unit that representing the alias.

This revision was automatically updated to reflect the committed changes.