[X86]: Add imp-uses of live regs when creating conditional tail calls (PR31257)

[hans]

Summary

This is a second stab at fixing PR31257 (the previous one was D29511). The patch is intended to be applied on top of a revert of r294348.

The idea is to mark registers which are live before the call, but potentially clobbered by it, as imp-used and imp-defined, as a way of keeping them alive. In reality, the registers are not clobbered unless the call is made, in which case it doesn't matter as control leaves the function.

This is the same technique that's used when creating predicated variants of instructions in IfConverter::CopyAndPredicateBlock.

[qcolombet]

lib/Target/X86/X86InstrInfo.cpp

5665

You'll need to add implicit-def as well otherwise the liveness will think later uses are referencing undef values, because this are still clobbered by the TC.
Basically, you've fixed the part of the live-range getting to the TC, but you have to fix part that leaves the TC.

[hans]

Adding imp-defs as well.

[hans]

test/CodeGen/X86/tail-call-conditional.mir

51

There is a lot of redundancy in the list of implicitly used registers. Do you know of a convenient way to clean it up?

[qcolombet]

LGTM

test/CodeGen/X86/tail-call-conditional.mir

51

I don't think we have a convenient way to clean that up other than eliminating the redundancies programmatically.
One way to do this is to walk through the list and remember the register with the largest size for each clobbered reg unit.
I believe we do something like that somewhere else, but I don't remember where on top of my head. What I am saying is that there may be opportunities to share some code.

lib/Target/X86/X86InstrInfo.cpp

unsigned Opc = TailCall.getOpcode() == X86::TCRETURNdi ? X86::TCRETURNdicc
                                                          : X86::TCRETURNdi64cc;
 
   auto MIB = BuildMI(MBB, I, MBB.findDebugLoc(I), get(Opc));
   MIB->addOperand(TailCall.getOperand(0)); // Destination.
   MIB.addImm(0); // Stack offset (not used).
   MIB->addOperand(BranchCond[0]); // Condition.
   MIB.copyImplicitOps(TailCall); // Regmask and (imp-used) parameters.
 
+  // Add implicit uses of all live regs potentially clobbered by the call. This
+  // way they are still live up to the call.
+  LivePhysRegs LiveRegs(&getRegisterInfo());
+  LiveRegs.addLiveOuts(MBB);
+  SmallVector<std::pair<unsigned, const MachineOperand *>, 8> Clobbers;
+  LiveRegs.stepForward(*MIB, Clobbers);
+  for (const auto &C : Clobbers) {
+    MIB->addOperand(MachineOperand::CreateReg(C.first, /*isDef*/ false,
+                                              /*isImplicit*/ true));
+  }
+
   I->eraseFromParent();
 }
 
 // Given a MBB and its TBB, find the FBB which was a fallthrough MBB (it may
 // not be a fallthrough MBB now due to layout changes). Return nullptr if the
 // fallthrough MBB cannot be identified.
 static MachineBasicBlock *getFallThroughMBB(MachineBasicBlock *MBB,
                                             MachineBasicBlock *TBB) {

test/CodeGen/X86/conditional-tailcall.ll

-; RUN: llc < %s -mtriple=i686-linux -show-mc-encoding | FileCheck %s
-; RUN: llc < %s -mtriple=x86_64-linux -show-mc-encoding | FileCheck %s
+; RUN: llc < %s -mtriple=i686-linux -show-mc-encoding | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK32
+; RUN: llc < %s -mtriple=x86_64-linux -show-mc-encoding | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK64
 
 declare void @foo()
 declare void @bar()
 
 define void @f(i32 %x, i32 %y) optsize {
 entry:
   %p = icmp eq i32 %x, %y
   br i1 %p, label %bb1, label %bb2

 
 ; Make sure machine block placement isn't confused by the conditional tail call,
 ; but sees that it can fall through to the next block.
 ; CHECK-LABEL: BlockPlacementTest
 ; CHECK: je baz
 ; CHECK-NOT: xor
 ; CHECK: ret
 }
+
+
+
+%"class.std::basic_string" = type { %"struct.std::basic_string<char, std::char_traits<char>, std::allocator<char> >::_Alloc_hider" }
+%"struct.std::basic_string<char, std::char_traits<char>, std::allocator<char> >::_Alloc_hider" = type { i8* }
+declare zeroext i1 @_Z20isValidIntegerSuffixN9__gnu_cxx17__normal_iteratorIPKcSsEES3_(i8*, i8*)
+
+define zeroext i1 @pr31257(%"class.std::basic_string"* nocapture readonly dereferenceable(8) %s) minsize {
+; CHECK-LABEL: pr31257
+entry:
+  %_M_p.i.i = getelementptr inbounds %"class.std::basic_string", %"class.std::basic_string"* %s, i64 0, i32 0, i32 0
+  %0 = load i8*, i8** %_M_p.i.i, align 8
+  %arrayidx.i.i.i54 = getelementptr inbounds i8, i8* %0, i64 -24
+  %_M_length.i.i55 = bitcast i8* %arrayidx.i.i.i54 to i64*
+  %1 = load i64, i64* %_M_length.i.i55, align 8
+  %add.ptr.i56 = getelementptr inbounds i8, i8* %0, i64 %1
+  br label %for.cond
+
+for.cond:                                         ; preds = %for.inc, %entry
+  %it.sroa.0.0 = phi i8* [ %0, %entry ], [ %incdec.ptr.i, %for.inc ]
+  %state.0 = phi i32 [ 0, %entry ], [ %state.1, %for.inc ]
+  %cmp.i = icmp eq i8* %it.sroa.0.0, %add.ptr.i56
+  br i1 %cmp.i, label %5, label %for.body
+
+for.body:                                         ; preds = %for.cond
+  switch i32 %state.0, label %for.inc [
+    i32 0, label %sw.bb
+    i32 1, label %sw.bb14
+    i32 2, label %sw.bb22
+  ]
+
+sw.bb:                                            ; preds = %for.body
+  %2 = load i8, i8* %it.sroa.0.0, align 1
+  switch i8 %2, label %if.else [
+    i8 43, label %for.inc
+    i8 45, label %for.inc
+  ]
+
+if.else:                                          ; preds = %sw.bb
+  %conv9 = zext i8 %2 to i32
+  %isdigittmp45 = add nsw i32 %conv9, -48
+  %isdigit46 = icmp ult i32 %isdigittmp45, 10
+  br i1 %isdigit46, label %for.inc, label %cleanup.thread.loopexit
+
+sw.bb14:                                          ; preds = %for.body
+  %3 = load i8, i8* %it.sroa.0.0, align 1
+  %conv16 = zext i8 %3 to i32
+  %isdigittmp43 = add nsw i32 %conv16, -48
+  %isdigit44 = icmp ult i32 %isdigittmp43, 10
+  br i1 %isdigit44, label %for.inc, label %cleanup.thread.loopexit
+
+sw.bb22:                                          ; preds = %for.body
+  %4 = load i8, i8* %it.sroa.0.0, align 1
+  %conv24 = zext i8 %4 to i32
+  %isdigittmp = add nsw i32 %conv24, -48
+  %isdigit = icmp ult i32 %isdigittmp, 10
+  br i1 %isdigit, label %for.inc, label %if.else28
+
+; Make sure Machine Copy Propagation doesn't delete the mov to %ecx becaue it
+; thinks the conditional tail call clobbers it.
+; CHECK64-LABEL: .LBB2_11:
+; CHECK64:       movzbl  (%rdi), %ecx
+; CHECK64-NEXT:  addl    $-48, %ecx
+; CHECK64-NEXT:  cmpl    $10, %ecx
+; CHECK64-NEXT:  movl    %r9d, %ecx
+; CHECK64-NEXT:  jae     _Z20isValidIntegerSuffixN9__gnu_cxx17__normal_iteratorIPKcSsEE
+
+if.else28:                                        ; preds = %sw.bb22
+  %call34 = tail call zeroext i1 @_Z20isValidIntegerSuffixN9__gnu_cxx17__normal_iteratorIPKcSsEES3_(i8* nonnull %it.sroa.0.0, i8* %add.ptr.i56)
+  br label %cleanup.thread
+
+for.inc:                                          ; preds = %sw.bb, %sw.bb, %sw.bb22, %sw.bb14, %if.else, %for.body
+  %state.1 = phi i32 [ %state.0, %for.body ], [ 1, %sw.bb ], [ 2, %if.else ], [ 2, %sw.bb14 ], [ 2, %sw.bb22 ], [ 1, %sw.bb ]
+  %incdec.ptr.i = getelementptr inbounds i8, i8* %it.sroa.0.0, i64 1
+  br label %for.cond
+
+; <label>:5:                                      ; preds = %for.cond
+  %cmp37 = icmp eq i32 %state.0, 2
+  br label %cleanup.thread
+
+cleanup.thread.loopexit:                          ; preds = %if.else, %sw.bb14
+  br label %cleanup.thread
+
+cleanup.thread:                                   ; preds = %cleanup.thread.loopexit, %if.else28, %5
+  %6 = phi i1 [ %cmp37, %5 ], [ %call34, %if.else28 ], [ false, %cleanup.thread.loopexit ]
+  ret i1 %6
+}

test/CodeGen/X86/tail-call-conditional.mir

body:             |
 
   ; CHECK: bb.1:
   ; CHECK-NEXT: successors: %bb.2({{[^)]+}}){{$}}
   ; CHECK-NEXT: liveins: %rax, %rsi
   ; CHECK-NEXT: {{^  $}}
   ; CHECK-NEXT: %rdi = COPY %rsi
   ; CHECK-NEXT: %rsi = COPY %rax
   ; CHECK-NEXT: CMP64ri8 %rax, 9, implicit-def %eflags
-  ; CHECK-NEXT: TCRETURNdi64cc @f1, 0, 3, csr_64, implicit %rsp, implicit %eflags, implicit %rsp, implicit %rdi, implicit %rsi
+  ; CHECK-NEXT: TCRETURNdi64cc @f1, 0, 3, csr_64, implicit %rsp, implicit %eflags, implicit %rsp, implicit %rdi, implicit %rsi, implicit %rax, implicit %sil, implicit %si, implicit %esi, implicit %rsi, implicit %dil, implicit %di, implicit %edi, implicit %rdi, implicit %ah, implicit %al, implicit %ax, implicit %eax
 
   bb.1:
     successors: %bb.2, %bb.3
     liveins: %rax, %rsi
 
     CMP64ri8 %rax, 9, implicit-def %eflags
     JA_1 %bb.3, implicit %eflags
     JMP_1 %bb.2