[GlobalISel] Tail call memcpy/memmove/memset even in the presence of copies

[jroelofs]

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

549

hm. This isn't quite right. It shouldn't be allowed to fire on this case:

%0:_(p0) = COPY $x1
%1:_(p0) = COPY $x0
%2:_(s64) = COPY $x2
G_MEMCPY %0(p0), %1(p0), %2(s64), 1 :: (store (s8)), (load (s8))
$x1 = COPY %0(p0)
RET_ReallyLR implicit $x0

[paquette]

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

504–509

This comment is out of date with this change.

514

Would be nice if this could be done for thisreturn callees in general, not just the memcpy/memmove/memset calls? Maybe a TODO?

549

I guess you need to check that the register is implicit on the return? So, you probably also have to check that there is a return in the block as well?

Also, what happens if there's more than one copy after the call?

e.g. I think this would do the wrong thing here too:

%0:_(p0) = COPY $x1
%1:_(p0) = COPY $x0
%2:_(s64) = COPY $x2
G_MEMCPY %0(p0), %1(p0), %2(s64), 1 :: (store (s8)), (load (s8))
$x1 = COPY %0(p0)
$x1 = COPY %1(p0)
RET_ReallyLR implicit $x1

[jroelofs]

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

514

The only other case is G_BZERO. Updated to give the default case an unreachable to make this clear.

549

I guess you need to check that the register is implicit on the return?

oh, right, thanks!

I believe the tailcall; copy; whatever; ret case is covered in the next line:

if (Next == MBB.instr_end() || TII.isTailCall(*Next) || !Next->isReturn())
   return false;

... since the preceding block skips at most a single copy instruction (+ debug insts) after the call. Added this as a test case for good measure.

[aemerson]

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

504–509

Why is it necessary to check for %0 = COPY $x0?

It seems the condition we need to check is that there's a write to $x0 (or whatever register is the return register) of the same vreg value as the first argument to G_MEMCPY.

G_MEMCPY %0, %1, %2
$x0 = COPY %0
RET_ReallyLR implicit $x0

[jroelofs]

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

504–509

On further thought, it isn't necessary. I was thinking that was needed to guarantee that %0's whole live range was assigned to $x0, but I don't think we actually need that.

[paquette]

So, this is always assuming we can treat G_MEMCPY and friends as thisreturn, right?

In D105671, @rjmccall mentioned that this may only be correct in terms of using it for ARC. Can we guarantee that we only do this for ARC here too?

[aemerson]

In D105382#2884618, @paquette wrote:

So, this is always assuming we can treat G_MEMCPY and friends as thisreturn, right?

In D105671, @rjmccall mentioned that this may only be correct in terms of using it for ARC. Can we guarantee that we only do this for ARC here too?

I read that comment to mean those only affected the objc functions, not memcpy.

[paquette]

ah ok then disregard

[jroelofs]

In D105382#2884618, @paquette wrote:

So, this is always assuming we can treat G_MEMCPY and friends as thisreturn, right?

The patch this one depends on makes that guarantee.

In D105671, @rjmccall mentioned that this may only be correct in terms of using it for ARC. Can we guarantee that we only do this for ARC here too?

The ARC thing was about doing the same for objc_retain.

[jroelofs]

Drop requirement that the destination argument be initially copied from the returned register.

[jroelofs]

I typo'd "Differentail" in the commit message.

This was landed here: a14b4e34a4569bb000ccdd4501628e1b891bcb38

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

#define RTLIBCASE(LibcallPrefix)                                               \
   case TargetOpcode::G_INTRINSIC_ROUNDEVEN:
     RTLIBCASE(ROUNDEVEN_F);
   }
   llvm_unreachable("Unknown libcall function");
 }
 
 /// True if an instruction is in tail position in its caller. Intended for
 /// legalizing libcalls as tail calls when possible.
-static bool isLibCallInTailPosition(const TargetInstrInfo &TII,
-                                    MachineInstr &MI) {
+static bool isLibCallInTailPosition(MachineInstr &MI,
+                                    const TargetInstrInfo &TII,
+                                    MachineRegisterInfo &MRI) {
   MachineBasicBlock &MBB = *MI.getParent();
   const Function &F = MBB.getParent()->getFunction();
 
   // Conservatively require the attributes of the call to match those of
   // the return. Ignore NoAlias and NonNull because they don't affect the
   // call sequence.
   AttributeList CallerAttrs = F.getAttributes();
   if (AttrBuilder(CallerAttrs, AttributeList::ReturnIndex)
           .removeAttribute(Attribute::NoAlias)
           .removeAttribute(Attribute::NonNull)
           .hasAttributes())
     return false;
 
   // It's not safe to eliminate the sign / zero extension of the return value.
   if (CallerAttrs.hasAttribute(AttributeList::ReturnIndex, Attribute::ZExt) ||
       CallerAttrs.hasAttribute(AttributeList::ReturnIndex, Attribute::SExt))
     return false;
 
-  // Only tail call if the following instruction is a standard return.
+  // Only tail call if the following instruction is a standard return or if we
+  // have a `thisreturn` callee, and a sequence like:
+  //
+  //   G_MEMCPY %0, %1, %2
+  //   $x0 = COPY %0
+  //   RET_ReallyLR implicit $x0
   auto Next = next_nodbg(MI.getIterator(), MBB.instr_end());
+  if (Next != MBB.instr_end() && Next->isCopy()) {
+    switch (MI.getOpcode()) {
+    default:
+      llvm_unreachable("unsupported opcode");
+    case TargetOpcode::G_BZERO:
+      return false;
+    case TargetOpcode::G_MEMCPY:
+    case TargetOpcode::G_MEMMOVE:
+    case TargetOpcode::G_MEMSET:
+      break;
+    }
+
+    Register VReg = MI.getOperand(0).getReg();
+    if (!VReg.isVirtual() || VReg != Next->getOperand(1).getReg())
+      return false;
+
+    Register PReg = Next->getOperand(0).getReg();
+    if (!PReg.isPhysical())
+      return false;
+
+    auto Ret = next_nodbg(Next, MBB.instr_end());
+    if (Ret == MBB.instr_end() || !Ret->isReturn())
+      return false;
+
+    if (Ret->getNumImplicitOperands() != 1)
+      return false;
+
+    if (PReg != Ret->getOperand(0).getReg())
+      return false;
+
+    // Skip over the COPY that we just validated.
+    Next = Ret;
+  }
+
   if (Next == MBB.instr_end() || TII.isTailCall(*Next) || !Next->isReturn())
     return false;
 
   return true;
 }
 
 LegalizerHelper::LegalizeResult
 llvm::createLibcall(MachineIRBuilder &MIRBuilder, const char *Name,
                     const CallLowering::ArgInfo &Result,
                     ArrayRef<CallLowering::ArgInfo> Args,
                     const CallingConv::ID CC) {
   auto &CLI = *MIRBuilder.getMF().getSubtarget().getCallLowering();
 
if (!Name) {
     return LegalizerHelper::UnableToLegalize;
   }
 
   CallLowering::CallLoweringInfo Info;
   Info.CallConv = TLI.getLibcallCallingConv(RTLibcall);
   Info.Callee = MachineOperand::CreateES(Name);
   Info.OrigRet = CallLowering::ArgInfo({0}, Type::getVoidTy(Ctx), 0);
   Info.IsTailCall = MI.getOperand(MI.getNumOperands() - 1).getImm() &&
-                    isLibCallInTailPosition(MIRBuilder.getTII(), MI);
+                    isLibCallInTailPosition(MI, MIRBuilder.getTII(), MRI);
 
   std::copy(Args.begin(), Args.end(), std::back_inserter(Info.OrigArgs));
   if (!CLI.lowerCall(MIRBuilder, Info))
     return LegalizerHelper::UnableToLegalize;
 
   if (Info.LoweredTailCall) {
     assert(Info.IsTailCall && "Lowered tail call when it wasn't a tail call?");
 
     // Check debug locations before removing the return.
     LocObserver.checkpoint(true);
 
     // We must have a return following the call (or debug insts) to get past
     // isLibCallInTailPosition.
     do {
       MachineInstr *Next = MI.getNextNode();
-      assert(Next && (Next->isReturn() || Next->isDebugInstr()) &&
+      assert(Next &&
+             (Next->isCopy() || Next->isReturn() || Next->isDebugInstr()) &&
              "Expected instr following MI to be return or debug inst?");
       // We lowered a tail call, so the call is now the return from the block.
       // Delete the old return.
       Next->eraseFromParent();
     } while (MI.getNextNode());
 
     // We expect to lose the debug location from the return.
     LocObserver.checkpoint(false);

llvm/test/CodeGen/AArch64/GlobalISel/legalize-memcpy-et-al.mir

 name:            no_tail_call
 tracksRegLiveness: true
 body:             |
   bb.1:
     liveins: $w2, $x0, $x1
 
     ; CHECK-LABEL: name: no_tail_call
     ; CHECK: liveins: $w2, $x0, $x1
-    ; CHECK: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
-    ; CHECK: [[COPY1:%[0-9]+]]:_(p0) = COPY $x1
+    ; CHECK: [[COPY:%[0-9]+]]:_(p0) = COPY $x1
+    ; CHECK: [[COPY1:%[0-9]+]]:_(p0) = COPY $x0
     ; CHECK: [[COPY2:%[0-9]+]]:_(s32) = COPY $w2
     ; CHECK: [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[COPY2]](s32)
     ; CHECK: ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
     ; CHECK: $x0 = COPY [[COPY]](p0)
     ; CHECK: $x1 = COPY [[COPY1]](p0)
     ; CHECK: $x2 = COPY [[ZEXT]](s64)
     ; CHECK: BL &memcpy, csr_aarch64_aapcs_thisreturn, implicit-def $lr, implicit $sp, implicit $x0, implicit $x1, implicit $x2
     ; CHECK: ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
     ; CHECK: $x0 = COPY [[ZEXT]](s64)
     ; CHECK: RET_ReallyLR implicit $x0
-    %0:_(p0) = COPY $x0
-    %1:_(p0) = COPY $x1
+    %0:_(p0) = COPY $x1
+    %1:_(p0) = COPY $x0
     %2:_(s32) = COPY $w2
     %3:_(s64) = G_ZEXT %2(s32)
     G_MEMCPY %0(p0), %1(p0), %3(s64), 1 :: (store unknown-size), (load unknown-size)
     $x0 = COPY %3
     RET_ReallyLR implicit $x0
 
 ...
 ---
bb.1:
     ; CHECK: TCRETURNdi &memset, 0, csr_aarch64_aapcs, implicit $sp
     %0:_(p0) = COPY $x0
     %1:_(p0) = COPY $x1
     %2:_(s32) = COPY $w2
     %4:_(s1) = G_CONSTANT i1 false
     %3:_(s64) = G_ZEXT %2(s32)
     G_MEMCPY %0(p0), %1(p0), %3(s64), 1 :: (store unknown-size), (load unknown-size)
     TCRETURNdi &memset, 0, csr_aarch64_aapcs, implicit $sp
+
+...
+---
+name:            tail_with_copy_ret
+alignment:       4
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+liveins:
+  - { reg: '$x0' }
+  - { reg: '$x1' }
+  - { reg: '$x2' }
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.1.entry:
+    liveins: $x0, $x1, $x2
+
+    ; CHECK-LABEL: name: tail_with_copy_ret
+    ; CHECK: liveins: $x0, $x1, $x2
+    ; CHECK: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
+    ; CHECK: [[COPY1:%[0-9]+]]:_(p0) = COPY $x1
+    ; CHECK: [[COPY2:%[0-9]+]]:_(s64) = COPY $x2
+    ; CHECK: $x0 = COPY [[COPY]](p0)
+    ; CHECK: $x1 = COPY [[COPY1]](p0)
+    ; CHECK: $x2 = COPY [[COPY2]](s64)
+    ; CHECK: TCRETURNdi &memcpy, 0, csr_aarch64_aapcs, implicit $sp, implicit $x0, implicit $x1, implicit $x2
+    %0:_(p0) = COPY $x0
+    %1:_(p0) = COPY $x1
+    %2:_(s64) = COPY $x2
+    G_MEMCPY %0(p0), %1(p0), %2(s64), 1 :: (store (s8)), (load (s8))
+    $x0 = COPY %0(p0)
+    RET_ReallyLR implicit $x0
+
+...
+---
+name:            dont_tc_mismatched_copies
+alignment:       4
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+liveins:
+  - { reg: '$x0' }
+  - { reg: '$x1' }
+  - { reg: '$x2' }
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.1.entry:
+    liveins: $x0, $x1, $x2
+
+    ; CHECK-LABEL: name: dont_tc_mismatched_copies
+    ; CHECK: liveins: $x0, $x1, $x2
+    ; CHECK: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
+    ; CHECK: [[COPY1:%[0-9]+]]:_(p0) = COPY $x1
+    ; CHECK: [[COPY2:%[0-9]+]]:_(s64) = COPY $x2
+    ; CHECK: ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+    ; CHECK: $x0 = COPY [[COPY]](p0)
+    ; CHECK: $x1 = COPY [[COPY1]](p0)
+    ; CHECK: $x2 = COPY [[COPY2]](s64)
+    ; CHECK: BL &memcpy, csr_aarch64_aapcs_thisreturn, implicit-def $lr, implicit $sp, implicit $x0, implicit $x1, implicit $x2
+    ; CHECK: ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+    ; CHECK: $x1 = COPY [[COPY]](p0)
+    ; CHECK: RET_ReallyLR implicit $x0
+    %0:_(p0) = COPY $x0
+    %1:_(p0) = COPY $x1
+    %2:_(s64) = COPY $x2
+    G_MEMCPY %0(p0), %1(p0), %2(s64), 1 :: (store (s8)), (load (s8))
+    $x1 = COPY %0(p0)
+    RET_ReallyLR implicit $x0
+
+...
+---
+name:            dont_tc_extra_copy
+alignment:       4
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+liveins:
+  - { reg: '$x0' }
+  - { reg: '$x1' }
+  - { reg: '$x2' }
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.1.entry:
+    liveins: $x0, $x1, $x2
+
+    ; CHECK-LABEL: name: dont_tc_extra_copy
+    ; CHECK: liveins: $x0, $x1, $x2
+    ; CHECK: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
+    ; CHECK: [[COPY1:%[0-9]+]]:_(p0) = COPY $x1
+    ; CHECK: [[COPY2:%[0-9]+]]:_(s64) = COPY $x2
+    ; CHECK: ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+    ; CHECK: $x0 = COPY [[COPY]](p0)
+    ; CHECK: $x1 = COPY [[COPY1]](p0)
+    ; CHECK: $x2 = COPY [[COPY2]](s64)
+    ; CHECK: BL &memcpy, csr_aarch64_aapcs_thisreturn, implicit-def $lr, implicit $sp, implicit $x0, implicit $x1, implicit $x2
+    ; CHECK: ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+    ; CHECK: $x0 = COPY [[COPY]](p0)
+    ; CHECK: $x0 = COPY [[COPY]](p0)
+    ; CHECK: RET_ReallyLR implicit $x0
+    %0:_(p0) = COPY $x0
+    %1:_(p0) = COPY $x1
+    %2:_(s64) = COPY $x2
+    G_MEMCPY %0(p0), %1(p0), %2(s64), 1 :: (store (s8)), (load (s8))
+    $x0 = COPY %0(p0)
+    $x0 = COPY %0(p0)
+    RET_ReallyLR implicit $x0
+
+...
+---
+name:            dont_tc_mismatched_ret
+alignment:       4
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+liveins:
+  - { reg: '$x0' }
+  - { reg: '$x1' }
+  - { reg: '$x2' }
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.1.entry:
+    liveins: $x0, $x1, $x2
+
+    ; CHECK-LABEL: name: dont_tc_mismatched_ret
+    ; CHECK: liveins: $x0, $x1, $x2
+    ; CHECK: [[COPY:%[0-9]+]]:_(p0) = COPY $x1
+    ; CHECK: [[COPY1:%[0-9]+]]:_(p0) = COPY $x0
+    ; CHECK: [[COPY2:%[0-9]+]]:_(s64) = COPY $x2
+    ; CHECK: ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+    ; CHECK: $x0 = COPY [[COPY]](p0)
+    ; CHECK: $x1 = COPY [[COPY1]](p0)
+    ; CHECK: $x2 = COPY [[COPY2]](s64)
+    ; CHECK: BL &memcpy, csr_aarch64_aapcs_thisreturn, implicit-def $lr, implicit $sp, implicit $x0, implicit $x1, implicit $x2
+    ; CHECK: ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+    ; CHECK: $x1 = COPY [[COPY]](p0)
+    ; CHECK: RET_ReallyLR implicit $x0
+    %0:_(p0) = COPY $x1
+    %1:_(p0) = COPY $x0
+    %2:_(s64) = COPY $x2
+    G_MEMCPY %0(p0), %1(p0), %2(s64), 1 :: (store (s8)), (load (s8))
+    $x1 = COPY %0(p0)
+    RET_ReallyLR implicit $x0
+
+...
+---
+name:            dont_tc_ret_void_copy
+alignment:       4
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+liveins:
+  - { reg: '$x0' }
+  - { reg: '$x1' }
+  - { reg: '$x2' }
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.1.entry:
+    liveins: $x0, $x1, $x2
+
+    ; CHECK-LABEL: name: dont_tc_ret_void_copy
+    ; CHECK: liveins: $x0, $x1, $x2
+    ; CHECK: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
+    ; CHECK: [[COPY1:%[0-9]+]]:_(p0) = COPY $x1
+    ; CHECK: [[COPY2:%[0-9]+]]:_(s64) = COPY $x2
+    ; CHECK: ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+    ; CHECK: $x0 = COPY [[COPY]](p0)
+    ; CHECK: $x1 = COPY [[COPY1]](p0)
+    ; CHECK: $x2 = COPY [[COPY2]](s64)
+    ; CHECK: BL &memcpy, csr_aarch64_aapcs_thisreturn, implicit-def $lr, implicit $sp, implicit $x0, implicit $x1, implicit $x2
+    ; CHECK: ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+    ; CHECK: $x0 = COPY [[COPY]](p0)
+    ; CHECK: RET_ReallyLR
+    %0:_(p0) = COPY $x0
+    %1:_(p0) = COPY $x1
+    %2:_(s64) = COPY $x2
+    G_MEMCPY %0(p0), %1(p0), %2(s64), 1 :: (store (s8)), (load (s8))
+    $x0 = COPY %0(p0)
+    RET_ReallyLR