[PowerPC] Fix CR Bit spill pseudo expansion

[nemanjai]

Summary

A client that uses LLVM for a very large code base has reported an issue where a spill of a CR bit causes a copy into another CR bit in the same CR field to be deleted as it appears to be dead.

The situation is illustrated in the MIR test case attached to this patch, but in a nutshell is the following:

1: $cr5lt = <some defining instr>
2: $cr5gt = COPY $cr1eq
3: SPILL_CRBIT $cr5lt
4: $cr1 = <some cr-field defining instr>
5: SPILL_CRBIT $cr5gt
...

In the current implementation, the COPY at 2 will be deleted since:

• The source of the copy cannot be forwarded to the spill at 5
• It is clobbered by the spill (due to the addition of the KILL instrucntion)

This patch just ensures this no longer happens by not emitting the KILL instruction and achieving what was presumably the purpose of that instruction by:

• Making the CR field that contains the bit being spilled an undef use
• Retaining the kill flag on the spilled bit through an implicit use

[hfinkel]

LGTM. This certainly looks like a better solution; the previous code was indeed trying to make sure that the containing CR field was defined before the mfocrf. Thanks!

lib/Target/PowerPC/PPCRegisterInfo.cpp

void PPCRegisterInfo::lowerCRBitSpilling(MachineBasicBlock::iterator II,
 
   bool LP64 = TM.isPPC64();
   const TargetRegisterClass *G8RC = &PPC::G8RCRegClass;
   const TargetRegisterClass *GPRC = &PPC::GPRCRegClass;
 
   unsigned Reg = MF.getRegInfo().createVirtualRegister(LP64 ? G8RC : GPRC);
   unsigned SrcReg = MI.getOperand(0).getReg();
 
-  BuildMI(MBB, II, dl, TII.get(TargetOpcode::KILL),
-          getCRFromCRBit(SrcReg))
-          .addReg(SrcReg, getKillRegState(MI.getOperand(0).isKill()));
-
+  // We need to move the CR field that contains the CR bit we are spilling.
+  // The super register may not be explicitly defined (i.e. it can be defined
+  // by a CR-logical that only defines the subreg) so we state that the CR
+  // field is undef. Also, in order to preserve the kill flag on the CR bit,
+  // we add it as an implicit use.
   BuildMI(MBB, II, dl, TII.get(LP64 ? PPC::MFOCRF8 : PPC::MFOCRF), Reg)
-      .addReg(getCRFromCRBit(SrcReg));
+      .addReg(getCRFromCRBit(SrcReg), RegState::Undef)
+      .addReg(SrcReg,
+              RegState::Implicit | getKillRegState(MI.getOperand(0).isKill()));
 
   // If the saved register wasn't CR0LT, shift the bits left so that the bit to
   // store is the first one. Mask all but that bit.
   unsigned Reg1 = Reg;
   Reg = MF.getRegInfo().createVirtualRegister(LP64 ? G8RC : GPRC);
 
   // rlwinm rA, rA, ShiftBits, 0, 0.
   BuildMI(MBB, II, dl, TII.get(LP64 ? PPC::RLWINM8 : PPC::RLWINM), Reg)

test/CodeGen/PowerPC/NoCRFieldRedefWhenSpillingCRBIT.mir

+# RUN: llc -mcpu=pwr8 -mtriple=powerpc64le-unknown-linux-gnu -start-after \
+# RUN:   virtregrewriter -ppc-asm-full-reg-names -verify-machineinstrs %s \
+# RUN:   -o - | FileCheck %s
+
+--- |
+  ; ModuleID = 'a.ll'
+  source_filename = "a.c"
+  target datalayout = "e-m:e-i64:64-n32:64"
+  target triple = "powerpc64le-unknown-linux-gnu"
+  
+  ; Function Attrs: nounwind
+  define void @test(i32 signext %a6, i32 signext %a7, i32 signext %a17) local_unnamed_addr #0 {
+  entry:
+    %cmp27 = icmp slt i32 %a6, %a7
+    %cmp29 = icmp sgt i32 %a6, %a17
+    %or.cond781 = or i1 %cmp27, %cmp29
+    tail call void asm sideeffect "# nothing", "~{cr0},~{cr1},~{cr2},~{cr3},~{cr4},~{cr5},~{cr6},~{cr7},~{memory}"() #1, !srcloc !1
+    br label %if.end326
+  
+  if.end326:                                        ; preds = %entry
+    br i1 %or.cond781, label %if.then330, label %if.end331
+  
+  if.then330:                                       ; preds = %if.end326
+    unreachable
+  
+  if.end331:                                        ; preds = %if.end326
+    ret void
+  }
+  
+  ; Function Attrs: nounwind
+  declare void @llvm.stackprotector(i8*, i8**) #1
+  
+  attributes #0 = { nounwind "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="ppc64le" "target-features"="+altivec,+bpermd,+crypto,+direct-move,+extdiv,+htm,+power8-vector,+vsx,-power9-vector,-qpx" "unsafe-fp-math"="false" "use-soft-float"="false" }
+  attributes #1 = { nounwind }
+  
+  !llvm.ident = !{!0}
+  
+  !0 = !{!"clang version 8.0.0 (trunk 349357)"}
+  !1 = !{i32 3373}
+
+...
+---
+name:            test
+alignment:       4
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+failedISel:      false
+tracksRegLiveness: true
+hasWinCFI:       false
+registers:       []
+liveins:         
+  - { reg: '$x3', virtual-reg: '' }
+  - { reg: '$x4', virtual-reg: '' }
+  - { reg: '$x5', virtual-reg: '' }
+frameInfo:       
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    4
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  maxCallFrameSize: 4294967295
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:      []
+stack:           
+  - { id: 0, name: '', type: spill-slot, offset: 0, size: 4, alignment: 4, 
+      stack-id: 0, callee-saved-register: '', callee-saved-restored: true, 
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  - { id: 1, name: '', type: spill-slot, offset: 0, size: 4, alignment: 4, 
+      stack-id: 0, callee-saved-register: '', callee-saved-restored: true, 
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  - { id: 2, name: '', type: spill-slot, offset: 0, size: 4, alignment: 4, 
+      stack-id: 0, callee-saved-register: '', callee-saved-restored: true, 
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+constants:       []
+body:             |
+  bb.0.entry:
+    liveins: $x3, $x4, $x5
+  
+    renamable $cr0 = CMPW renamable $r3, renamable $r4, implicit $x4
+    renamable $cr1 = CMPW renamable $r3, renamable $r5, implicit $x5, implicit killed $x3
+    renamable $cr5lt = CRNOR renamable $cr0lt, renamable $cr1gt, implicit killed $cr0
+    renamable $cr5gt = COPY renamable $cr1gt, implicit $cr1
+    ; CHECK: crnor 4*cr5+lt, lt, 4*cr1+gt
+    ; CHECK: cror 4*cr5+gt, 4*cr1+gt, 4*cr1+gt
+    SPILL_CRBIT killed renamable $cr5lt, 0, %stack.0 :: (store 4 into %stack.0)
+    renamable $cr1 = CMPW renamable $r4, renamable $r5, implicit killed $x5, implicit killed $x4
+    SPILL_CRBIT killed renamable $cr5gt, 0, %stack.1 :: (store 4 into %stack.1)
+    SPILL_CRBIT killed renamable $cr1gt, 0, %stack.2 :: (store 4 into %stack.2)
+    INLINEASM &"# nothing", 25, 12, implicit-def dead early-clobber $cr0, 12, implicit-def dead early-clobber $cr1, 12, implicit-def dead early-clobber $cr2, 12, implicit-def dead early-clobber $cr3, 12, implicit-def dead early-clobber $cr4, 12, implicit-def dead early-clobber $cr5, 12, implicit-def dead early-clobber $cr6, 12, implicit-def dead early-clobber $cr7, !1
+    BLR8 implicit $lr8, implicit $rm
+  
+  bb.1.if.end326:
+    successors: %bb.2(0x00000001), %bb.3(0x7fffffff)
+  
+    renamable $cr5lt = RESTORE_CRBIT 0, %stack.0 :: (load 4 from %stack.0)
+    renamable $cr5gt = RESTORE_CRBIT 0, %stack.1 :: (load 4 from %stack.1)
+    renamable $cr5lt = CROR killed renamable $cr5lt, killed renamable $cr5gt
+    BCn killed renamable $cr5lt, %bb.3
+    B %bb.2
+  
+  bb.2.if.then330:
+    successors: 
+  
+  
+  bb.3.if.end331:
+    BLR8 implicit $lr8, implicit $rm
+
+...