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

[craig.topper]

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

[spatel]

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

266

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

13

Remove FIXME note.

[spatel]

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

[craig.topper]

Use a lambda

[spatel]

LGTM

llvm/lib/CodeGen/CriticalAntiDepBreaker.cpp

void CriticalAntiDepBreaker::ScanInstruction(MachineInstr &MI, unsigned Count) {
   assert(!MI.isKill() && "Attempting to scan a kill instruction");
 
   if (!TII->isPredicated(MI)) {
     // Predicated defs are modeled as read + write, i.e. similar to two
     // address updates.
     for (unsigned i = 0, e = MI.getNumOperands(); i != e; ++i) {
       MachineOperand &MO = MI.getOperand(i);
 
-      if (MO.isRegMask())
-        for (unsigned i = 0, e = TRI->getNumRegs(); i != e; ++i)
+      if (MO.isRegMask()) {
+        for (unsigned i = 0, e = TRI->getNumRegs(); i != e; ++i) {
           if (MO.clobbersPhysReg(i)) {
+            // Make sure all subregisters are also clobbered.
+            bool PreservesSubReg = false;
+            for (MCSubRegIterator SRI(i, TRI, true); SRI.isValid(); ++SRI) {
+              if (!MO.clobbersPhysReg(*SRI)) {
+                PreservesSubReg = true;
+                break;
+              }
+            }
+
+            if (!PreservesSubReg) {
               DefIndices[i] = Count;
               KillIndices[i] = ~0u;
               KeepRegs.reset(i);
               Classes[i] = nullptr;
               RegRefs.erase(i);
             }
+          }
+        }
+      }
 
       if (!MO.isReg()) continue;
       Register Reg = MO.getReg();
       if (Reg == 0) continue;
       if (!MO.isDef()) continue;
 
       // Ignore two-addr defs.
       if (MI.isRegTiedToUseOperand(i))

llvm/test/CodeGen/X86/pr44140.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=znver1 | FileCheck %s
 
 define win64cc void @opaque() {
 ; CHECK-LABEL: opaque:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    retq
   ret void
 }
 
 ; We need xmm6 to be live from the loop header across all iterations of the loop.
 ; We shouldn't clobber ymm6 inside the loop.
 ; FIXME: We currently clobber ymm6
 define i32 @main() {
 ; CHECK-LABEL: main:
 ; CHECK:       # %bb.0: # %start
 ; CHECK-NEXT:    subq $584, %rsp # imm = 0x248
 ; CHECK-NEXT:    .cfi_def_cfa_offset 592
 ; CHECK-NEXT:    vmovaps {{.*#+}} xmm6 = [1010101010101010101,2020202020202020202]
 ; CHECK-NEXT:    xorl %esi, %esi
 ; CHECK-NEXT:    .p2align 4, 0x90
 ; CHECK-NEXT:  .LBB1_1: # %fake-loop
 ; CHECK-NEXT:    # =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:    vmovups {{[0-9]+}}(%rsp), %ymm0
 ; CHECK-NEXT:    vmovups {{[0-9]+}}(%rsp), %ymm1
-; CHECK-NEXT:    vmovups {{[0-9]+}}(%rsp), %ymm6
+; CHECK-NEXT:    vmovups {{[0-9]+}}(%rsp), %ymm7
 ; CHECK-NEXT:    vmovups {{[0-9]+}}(%rsp), %ymm2
 ; CHECK-NEXT:    vmovups {{[0-9]+}}(%rsp), %ymm3
 ; CHECK-NEXT:    vmovups %ymm0, {{[0-9]+}}(%rsp)
 ; CHECK-NEXT:    vmovups %ymm1, {{[0-9]+}}(%rsp)
 ; CHECK-NEXT:    vmovups {{[0-9]+}}(%rsp), %ymm1
 ; CHECK-NEXT:    vmovups %ymm3, {{[0-9]+}}(%rsp)
 ; CHECK-NEXT:    vmovups %ymm2, {{[0-9]+}}(%rsp)
-; CHECK-NEXT:    vmovups %ymm6, {{[0-9]+}}(%rsp)
+; CHECK-NEXT:    vmovups %ymm7, {{[0-9]+}}(%rsp)
 ; CHECK-NEXT:    vmovups %ymm3, {{[0-9]+}}(%rsp)
 ; CHECK-NEXT:    vmovups %ymm2, {{[0-9]+}}(%rsp)
-; CHECK-NEXT:    vmovups %ymm6, {{[0-9]+}}(%rsp)
+; CHECK-NEXT:    vmovups %ymm7, {{[0-9]+}}(%rsp)
 ; CHECK-NEXT:    vmovups %ymm1, {{[0-9]+}}(%rsp)
 ; CHECK-NEXT:    vmovups %ymm1, {{[0-9]+}}(%rsp)
 ; CHECK-NEXT:    vmovups {{[0-9]+}}(%rsp), %ymm5
 ; CHECK-NEXT:    vmovups {{[0-9]+}}(%rsp), %ymm4
 ; CHECK-NEXT:    vmovups %ymm5, {{[0-9]+}}(%rsp)
 ; CHECK-NEXT:    vmovups %ymm4, {{[0-9]+}}(%rsp)
 ; CHECK-NEXT:    vzeroupper
 ; CHECK-NEXT:    callq opaque