[GlobalISel] Mark memcpy/memmove/memset as thisreturn

[jroelofs]

Summary

https://clang.godbolt.org/z/PTn4qha8v

[paquette]

SDAG doesn't seem to mark the call as returned with the same example. Is this a new optimization?

https://clang.godbolt.org/z/K49z51zoo

IIRC thisreturn comes up as a result of the returned attribute in IR, right? Should the frontend be marking memory functions as returned instead? (I guess that we'd need a patch like this either way...?)

[arsenm]

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

594

Why change to unreachable? This is an unrelated change.

[aemerson]

In D105370#2856111, @paquette wrote:

SDAG doesn't seem to mark the call as returned with the same example. Is this a new optimization?

https://clang.godbolt.org/z/K49z51zoo

IIRC thisreturn comes up as a result of the returned attribute in IR, right? Should the frontend be marking memory functions as returned instead? (I guess that we'd need a patch like this either way...?)

Godbolt is using c++ by default so we need to extern C the source: https://clang.godbolt.org/z/594M46vYs

But it seems the difference is that SDAG can tail call it while GISel doesn't.

[jroelofs]

In D105370#2856111, @paquette wrote:

SDAG doesn't seem to mark the call as returned with the same example.

It doesn't, but it must know something about this property since it doesn't restore $x0 afterward.

Is this a new optimization?

rdar://77466123

https://clang.godbolt.org/z/K49z51zoo

IIRC thisreturn comes up as a result of the returned attribute in IR, right? Should the frontend be marking memory functions as returned instead? (I guess that we'd need a patch like this either way...?)

Exactly. The thing that's special here is that these three functions have their own GISel Opcodes, which don't have that returned behavior that the call had, so it wasn't being transferred automatically like it would for other calls with the attribute.

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

594

The one call site only passes these four opcodes, so I figured this was a useful drive-by improvement. Happy to take it out if you think it should stay generic.

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

llvm::createMemLibcall(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI,
   RTLIB::Libcall RTLibcall;
   unsigned Opc = MI.getOpcode();
   switch (Opc) {
   case TargetOpcode::G_BZERO:
     RTLibcall = RTLIB::BZERO;
     break;
   case TargetOpcode::G_MEMCPY:
     RTLibcall = RTLIB::MEMCPY;
+    Args[0].Flags[0].setReturned();
     break;
   case TargetOpcode::G_MEMMOVE:
     RTLibcall = RTLIB::MEMMOVE;
+    Args[0].Flags[0].setReturned();
     break;
   case TargetOpcode::G_MEMSET:
     RTLibcall = RTLIB::MEMSET;
+    Args[0].Flags[0].setReturned();
     break;
   default:
-    return LegalizerHelper::UnableToLegalize;
+    llvm_unreachable("unsupported opcode");
   }
   const char *Name = TLI.getLibcallName(RTLibcall);
 
   // Unsupported libcall on the target.
   if (!Name) {
     LLVM_DEBUG(dbgs() << ".. .. Could not find libcall name for "
                       << MIRBuilder.getTII().getName(Opc) << "\n");
     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);
 
   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.

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

bb.1:
     ; CHECK: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
     ; CHECK: [[COPY1:%[0-9]+]]:_(p0) = COPY $x1
     ; 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, implicit-def $lr, implicit $sp, implicit $x0, implicit $x1, implicit $x2
+    ; 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: RET_ReallyLR
     %0:_(p0) = COPY $x0
     %1:_(p0) = COPY $x1
     %2:_(s32) = COPY $w2
     %3:_(s64) = G_ZEXT %2(s32)
     G_MEMCPY %0(p0), %1(p0), %3(s64), 0 :: (store unknown-size), (load unknown-size)
     RET_ReallyLR
bb.1:
     ; CHECK: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
     ; CHECK: [[COPY1:%[0-9]+]]:_(p0) = COPY $x1
     ; 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 &memmove, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit $x0, implicit $x1, implicit $x2
+    ; CHECK: BL &memmove, 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: RET_ReallyLR
     %0:_(p0) = COPY $x0
     %1:_(p0) = COPY $x1
     %2:_(s32) = COPY $w2
     %3:_(s64) = G_ZEXT %2(s32)
     G_MEMMOVE %0(p0), %1(p0), %3(s64), 0 :: (store unknown-size), (load unknown-size)
     RET_ReallyLR
bb.1:
     ; CHECK: [[COPY1:%[0-9]+]]:_(s32) = COPY $w1
     ; 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: [[COPY3:%[0-9]+]]:_(s32) = COPY [[COPY1]](s32)
     ; CHECK: $w1 = COPY [[COPY3]](s32)
     ; CHECK: $x2 = COPY [[ZEXT]](s64)
-    ; CHECK: BL &memset, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit $x0, implicit $w1, implicit $x2
+    ; CHECK: BL &memset, csr_aarch64_aapcs_thisreturn, implicit-def $lr, implicit $sp, implicit $x0, implicit $w1, implicit $x2
     ; CHECK: ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
     ; CHECK: RET_ReallyLR
     %0:_(p0) = COPY $x0
     %1:_(s32) = COPY $w1
     %2:_(s32) = COPY $w2
     %3:_(s8) = G_TRUNC %1(s32)
     %4:_(s64) = G_ZEXT %2(s32)
     G_MEMSET %0(p0), %3(s8), %4(s64), 0 :: (store unknown-size)
bb.1:
     ; CHECK: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
     ; CHECK: [[COPY1:%[0-9]+]]:_(p0) = COPY $x1
     ; 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, implicit-def $lr, implicit $sp, implicit $x0, implicit $x1, implicit $x2
+    ; 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
     %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)
bb.1:
     ; CHECK: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
     ; CHECK: [[COPY1:%[0-9]+]]:_(p0) = COPY $x1
     ; 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, implicit-def $lr, implicit $sp, implicit $x0, implicit $x1, implicit $x2
+    ; 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: 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

llvm/test/CodeGen/AArch64/GlobalISel/legalize-memlib-debug-loc.mir

liveins:
   - { reg: '$w1' }
   - { reg: '$w2' }
 body:             |
   bb.1.entry:
     liveins: $w1, $w2, $x0
 
     ; We're checking that the BL call has the debug loc of the original intrinsic call.
     ; CHECK-LABEL: name: test_memset_debug
-    ; CHECK: BL &memset, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit $x0, implicit $w1, implicit $x2, debug-location !11
+    ; CHECK: BL &memset, csr_aarch64_aapcs_thisreturn, implicit-def $lr, implicit $sp, implicit $x0, implicit $w1, implicit $x2, debug-location !11
     ; CHECK: RET_ReallyLR debug-location !12
     %0:_(p0) = COPY $x0
     %1:_(s32) = COPY $w1
     %2:_(s32) = COPY $w2
     %3:_(s64) = G_ZEXT %2(s32), debug-location !11
     %4:_(s8) = G_TRUNC %1(s32), debug-location !11
     G_MEMSET %0(p0), %4(s8), %3(s64), 0, debug-location !11 :: (store (s8) into %ir.ptr)
     RET_ReallyLR debug-location !12
 
 ...