Page MenuHomePhabricator

[CriticalAntiDepBreaker] Teach the regmask clobber check to check if any subregister is preserved before considering the super register clobbered
ClosedPublic

Authored by craig.topper on Mon, Nov 25, 3:29 PM.

Details

Summary

X86 has some calling conventions where bits 127:0 of a vector register are callee saved, but the upper bits aren't. Previously we could detect that the full ymm register was clobbered when the xmm portion was really preserved. This patch checks the subregisters to make sure they aren't preserved.

Fixes PR44140

Diff Detail

Event Timeline

craig.topper created this revision.Mon, Nov 25, 3:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Nov 25, 3:29 PM
Herald added a subscriber: hiraditya. · View Herald Transcript

I think AggressiveAntiDepBreaker::HandleLastUse() is doing a similar check already, but it would be good if someone can confirm that.

llvm/lib/CodeGen/CriticalAntiDepBreaker.cpp
264–278

Is it useful to make this logic available to everyone? If so, this becomes a 1-line patch that would call something like:

bool
MachineOperand::clobbersPhysRegAndSubRegs(unsigned PhysReg,
                                          const TargetRegisterInfo &TRI) const {
  for (MCSubRegIterator SRI(PhysReg, &TRI, true); SRI.isValid(); ++SRI)
    if (!clobbersPhysReg(*SRI))
      return false;

  return true;
}

Even if that's not worth adding, it would be easier reading if we had something like that as a static or lambda in this pass.

llvm/test/CodeGen/X86/pr44140.ll
12–13

Remove FIXME note.

Searching on earlier diffs leads to:
rL227311
D18448
...so adding some more potential reviewers based on that.

Use a lambda

nikic added a subscriber: nikic.Tue, Nov 26, 11:54 PM
spatel accepted this revision.Wed, Nov 27, 7:23 AM

LGTM

This revision is now accepted and ready to land.Wed, Nov 27, 7:23 AM
This revision was automatically updated to reflect the committed changes.