[CodeGen] Fix nomerge attribute not working in tail calls.

[zequanwu]

Summary

In D79537, nomerge was made to only apply to non-tail calls. This fixes it by also applying it to tail calls.

For ARM, I only made the new MI to inherit the flag under TCRETURNdi and TCRETURNri, because that's the place tail calls got replaced. Not sure if there's any other place needed.

Fixes #61545.

[hans]

Looks good as far as I can tell, but hopefully Craig who knows backends better can also take a look.

Maybe add to the description that this is a follow-up to D79537 which extends support to tail calls. (And do we need to do something for globalisel, or can that be left as a todo?)

[zequanwu]

(And do we need to do something for globalisel, or can that be left as a todo?)

I don't know about globalisel. For the repro, this patch fixes that.

[craig.topper]

llvm/test/CodeGen/AArch64/nomerge.ll

23

This should be in the body of the @foo_tail function. Did you run utils/update_llc_test_checks.py?

llvm/test/CodeGen/ARM/nomerge.ll

69

These should be in the body of @foo_tail

[zequanwu]

Update all tests by running llvm/utils/update_llc_test_checks.py.

[zequanwu]

llvm/test/CodeGen/AArch64/nomerge.ll

23

Updated. I didn't realize those CHECKs were generated by the script.

[jrtc27]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

6308

Leave whitespace not in the code you're touching alone

llvm/test/CodeGen/AArch64/nomerge.ll

22

Where did this come from?

41

Precommit all these?

llvm/test/CodeGen/ARM/nomerge.ll

2

This conversion should likely be precommitted to highlight the actual diff

llvm/test/CodeGen/RISCV/nomerge.ll

2

Ugh, D79537 added this without getting a RISC-V contributor's review (Craig wasn't yet one)... like the others, convert this separately

8

These tests could benefit from nounwind to trim irrelevant CFI directives

[zequanwu]

• Reverted white spaces changes.
• Add nounwind for tests to trim CFI directives.

llvm/test/CodeGen/AArch64/nomerge.ll

41

Can you elaborate on what does precommit mean? Do you mean commit on foo_tail functions without CHECKs before this patch?

[jrtc27]

llvm/test/CodeGen/AArch64/nomerge.ll

41

Commit the tests with CHECK lines generated from the compiler without this codegen change applied

[zequanwu]

Rebase.

llvm/test/CodeGen/AArch64/nomerge.ll

41

Landed at https://reviews.llvm.org/rG321d02cc6b6a1116c227c172249b5cacf4d0deba. Rebased this patch.

[zequanwu]

Ping.

[jrtc27]

llvm/test/CodeGen/RISCV/nomerge.ll

43

?

[zequanwu]

Address comment.

[zequanwu]

Ping.

[rnk]

This looks good to me.

I thought perhaps it would be shorter and simpler to have addNoMergeInfo take and return the SDValue which is being marked to avoid the need for new SDValue variables, but I don't think that's essential.

@jrtc27 , are your concerns addressed?

Regarding globalisel, @zequanwu , can you file an issue for implementing nomerge support in globalisel? It's probably easiest to construct an example with the AArch64 backend.

[jrtc27]

In D146749#4332699, @rnk wrote:

This looks good to me.

I thought perhaps it would be shorter and simpler to have addNoMergeInfo take and return the SDValue which is being marked to avoid the need for new SDValue variables, but I don't think that's essential.

@jrtc27 , are your concerns addressed?

Regarding globalisel, @zequanwu , can you file an issue for implementing nomerge support in globalisel? It's probably easiest to construct an example with the AArch64 backend.

Yes, the tests are much cleaner now, both in content and in reviewability of the diff, thanks.

[zequanwu]

Regarding globalisel, @zequanwu , can you file an issue for implementing nomerge support in globalisel? It's probably easiest to construct an example with the AArch64 backend.

Filed at https://github.com/llvm/llvm-project/issues/62641

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

AArch64TargetLowering::LowerCall(CallLoweringInfo &CLI,
   // actual call instruction.
   if (IsTailCall) {
     MF.getFrameInfo().setHasTailCall();
     SDValue Ret = DAG.getNode(AArch64ISD::TC_RETURN, DL, NodeTys, Ops);
 
     if (IsCFICall)
       Ret.getNode()->setCFIType(CLI.CFIType->getZExtValue());
 
+    DAG.addNoMergeSiteInfo(Ret.getNode(), CLI.NoMerge);
     DAG.addCallSiteInfo(Ret.getNode(), std::move(CSInfo));
     return Ret;
   }
 
   unsigned CallOpc = AArch64ISD::CALL;
   // Calls with operand bundle "clang.arc.attachedcall" are special. They should
   // be expanded to the call, directly followed by a special marker sequence and
   // a call to an ObjC library function.  Use CALL_RVMARKER to do that.

llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp

case ARM::TCRETURNri: {
       auto NewMI = std::prev(MBBI);
       for (unsigned i = 2, e = MBBI->getNumOperands(); i != e; ++i)
         NewMI->addOperand(MBBI->getOperand(i));
 
 
       // Update call site info and delete the pseudo instruction TCRETURN.
       if (MI.isCandidateForCallSiteEntry())
         MI.getMF()->moveCallSiteInfo(&MI, &*NewMI);
+      // Copy nomerge flag over to new instruction.
+      if (MI.getFlag(MachineInstr::NoMerge))
+        NewMI->setFlag(MachineInstr::NoMerge);
       MBB.erase(MBBI);
 
       MBBI = NewMI;
       return true;
     }
     case ARM::tBXNS_RET: {
       // For v8.0-M.Main we need to authenticate LR before clearing FPRs, which
       // uses R12 as a scratch register.

llvm/lib/Target/ARM/ARMISelLowering.cpp

ARMTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
 
   if (InGlue.getNode())
     Ops.push_back(InGlue);
 
   SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
   if (isTailCall) {
     MF.getFrameInfo().setHasTailCall();
     SDValue Ret = DAG.getNode(ARMISD::TC_RETURN, dl, NodeTys, Ops);
+    DAG.addNoMergeSiteInfo(Ret.getNode(), CLI.NoMerge);
     DAG.addCallSiteInfo(Ret.getNode(), std::move(CSInfo));
     return Ret;
   }
 
   // Returns a chain and a flag for retval copy to use.
   Chain = DAG.getNode(CallOpc, dl, NodeTys, Ops);
   DAG.addNoMergeSiteInfo(Chain.getNode(), CLI.NoMerge);
   InGlue = Chain.getValue(1);

llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp

LoongArchTargetLowering::LowerCall(CallLoweringInfo &CLI,
   if (Glue.getNode())
     Ops.push_back(Glue);
 
   // Emit the call.
   SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
 
   if (IsTailCall) {
     MF.getFrameInfo().setHasTailCall();
-    return DAG.getNode(LoongArchISD::TAIL, DL, NodeTys, Ops);
+    SDValue Ret = DAG.getNode(LoongArchISD::TAIL, DL, NodeTys, Ops);
+    DAG.addNoMergeSiteInfo(Ret.getNode(), CLI.NoMerge);
+    return Ret;
   }
 
   Chain = DAG.getNode(LoongArchISD::CALL, DL, NodeTys, Ops);
   DAG.addNoMergeSiteInfo(Chain.getNode(), CLI.NoMerge);
   Glue = Chain.getValue(1);
 
   // Mark the end of the call, which is glued to the call itself.
   Chain = DAG.getCALLSEQ_END(Chain, NumBytes, 0, Glue, DL);

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

assert(((Callee.getOpcode() == ISD::Register &&
             (CFlags.IsIndirect && Subtarget.isUsingPCRelativeCalls())) &&
            "Expecting a global address, external symbol, absolute value, "
            "register or an indirect tail call when PC Relative calls are "
            "used.");
     // PC Relative calls also use TC_RETURN as the way to mark tail calls.
     assert(CallOpc == PPCISD::TC_RETURN &&
            "Unexpected call opcode for a tail call.");
     DAG.getMachineFunction().getFrameInfo().setHasTailCall();
-    return DAG.getNode(CallOpc, dl, MVT::Other, Ops);
+    SDValue Ret = DAG.getNode(CallOpc, dl, MVT::Other, Ops);
+    DAG.addNoMergeSiteInfo(Ret.getNode(), CFlags.NoMerge);
+    return Ret;
   }
 
   std::array<EVT, 2> ReturnTypes = {{MVT::Other, MVT::Glue}};
   Chain = DAG.getNode(CallOpc, dl, ReturnTypes, Ops);
   DAG.addNoMergeSiteInfo(Chain.getNode(), CFlags.NoMerge);
   Glue = Chain.getValue(1);
 
   // When performing tail call optimization the callee pops its arguments off
if (Flags.isByVal()) {
           AddPtr = DAG.getNode(ISD::ADD, dl, PtrVT, PtrOff, Const);
         }
         Chain = CallSeqStart = createMemcpyOutsideCallSeq(Arg, AddPtr,
                                                           CallSeqStart,
                                                           Flags, DAG, dl);
         ArgOffset += PtrByteSize;
         continue;
       }
       // Copy the object to parameter save area if it can not be entirely passed 
       // by registers.
       // FIXME: we only need to copy the parts which need to be passed in
       // parameter save area. For the parts passed by registers, we don't need
       // to copy them to the stack although we need to allocate space for them
       // in parameter save area.
       if ((NumGPRs - GPR_idx) * PtrByteSize < Size)
         Chain = CallSeqStart = createMemcpyOutsideCallSeq(Arg, PtrOff,
                                                           CallSeqStart,

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

SDValue RISCVTargetLowering::LowerCall(CallLoweringInfo &CLI,
   if (Glue.getNode())
     Ops.push_back(Glue);
 
   // Emit the call.
   SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
 
   if (IsTailCall) {
     MF.getFrameInfo().setHasTailCall();
-    return DAG.getNode(RISCVISD::TAIL, DL, NodeTys, Ops);
+    SDValue Ret = DAG.getNode(RISCVISD::TAIL, DL, NodeTys, Ops);
+    DAG.addNoMergeSiteInfo(Ret.getNode(), CLI.NoMerge);
+    return Ret;
   }
 
   Chain = DAG.getNode(RISCVISD::CALL, DL, NodeTys, Ops);
   DAG.addNoMergeSiteInfo(Chain.getNode(), CLI.NoMerge);
   Glue = Chain.getValue(1);
 
   // Mark the end of the call, which is glued to the call itself.
   Chain = DAG.getCALLSEQ_END(Chain, NumBytes, 0, Glue, DL);

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

SystemZTargetLowering::LowerCall(CallLoweringInfo &CLI,
   Ops.push_back(DAG.getRegisterMask(Mask));
 
   // Glue the call to the argument copies, if any.
   if (Glue.getNode())
     Ops.push_back(Glue);
 
   // Emit the call.
   SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
-  if (IsTailCall)
-    return DAG.getNode(SystemZISD::SIBCALL, DL, NodeTys, Ops);
+  if (IsTailCall) {
+    SDValue Ret = DAG.getNode(SystemZISD::SIBCALL, DL, NodeTys, Ops);
+    DAG.addNoMergeSiteInfo(Ret.getNode(), CLI.NoMerge);
+    return Ret;
+  }
   Chain = DAG.getNode(SystemZISD::CALL, DL, NodeTys, Ops);
   DAG.addNoMergeSiteInfo(Chain.getNode(), CLI.NoMerge);
   Glue = Chain.getValue(1);
 
   // Mark the end of the call, which is glued to the call itself.
   Chain = DAG.getCALLSEQ_END(Chain, NumBytes, 0, Glue, DL);
   Glue = Chain.getValue(1);
 

llvm/lib/Target/X86/X86ISelLowering.cpp

if (isTailCall) {
     // should be computed from returns not tail calls.  Consider a void
     // function making a tail call to a function returning int.
     MF.getFrameInfo().setHasTailCall();
     SDValue Ret = DAG.getNode(X86ISD::TC_RETURN, dl, NodeTys, Ops);
 
     if (IsCFICall)
       Ret.getNode()->setCFIType(CLI.CFIType->getZExtValue());
 
+    DAG.addNoMergeSiteInfo(Ret.getNode(), CLI.NoMerge);
     DAG.addCallSiteInfo(Ret.getNode(), std::move(CSInfo));
     return Ret;
   }
 
   if (HasNoCfCheck && IsCFProtectionSupported && IsIndirectCall) {
     Chain = DAG.getNode(X86ISD::NT_CALL, dl, NodeTys, Ops);
   } else if (CLI.CB && objcarc::hasAttachedCallOpBundle(CLI.CB)) {
     // Calls with a "clang.arc.attachedcall" bundle are special. They should be

llvm/test/CodeGen/AArch64/nomerge.ll

 ; CHECK-NEXT:    bl bar
 ; CHECK-NEXT:    ldr x30, [sp], #16 // 8-byte Folded Reload
 ; CHECK-NEXT:    b bar
 ; CHECK-NEXT:  .LBB0_3: // %if.then2
 ; CHECK-NEXT:    bl bar
 ; CHECK-NEXT:  .LBB0_4: // %if.end3
 ; CHECK-NEXT:    ldr x30, [sp], #16 // 8-byte Folded Reload
 ; CHECK-NEXT:    b bar
 entry:
   switch i32 %i, label %if.end3 [
     i32 5, label %if.then
     i32 7, label %if.then2
   ]
 
 if.then:
   tail call void @bar() #0
   br label %if.end3
 
 if.then2:
   tail call void @bar() #0
   br label %if.end3
 
 if.end3:
   tail call void @bar() #0
   ret void
 }
 
 define void @foo_tail(i1 %i) nounwind {
 ; CHECK-LABEL: foo_tail:
 ; CHECK:       // %bb.0: // %entry
+; CHECK-NEXT:    tbz w0, #0, .LBB1_2
+; CHECK-NEXT:  // %bb.1: // %if.then
+; CHECK-NEXT:    b bar
+; CHECK-NEXT:  .LBB1_2: // %if.else
 ; CHECK-NEXT:    b bar
 entry:
   br i1 %i, label %if.then, label %if.else
 
 if.then:
   tail call void @bar() #0
   br label %if.end
 

llvm/test/CodeGen/ARM/nomerge.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
 ; RUN: llc < %s -mtriple=arm -o - | FileCheck %s
 
 define void @foo(i32 %i) nounwind {
 ; CHECK-LABEL: foo:
 ; CHECK:       @ %bb.0: @ %entry
 ; CHECK-NEXT:    push {r11, lr}
 ; CHECK-NEXT:    cmp r0, #7
 ; CHECK-NEXT:    beq .LBB0_3
 ; CHECK-NEXT:  @ %bb.1: @ %entry
if.end3:
   tail call void @bar() #0
   ret void
 }
 
 define void @foo_tail(i1 %i) nounwind {
 ; CHECK-LABEL: foo_tail:
 ; CHECK:       @ %bb.0: @ %entry
 ; CHECK-NEXT:    tst r0, #1
+; CHECK-NEXT:    beq .LBB1_2
+; CHECK-NEXT:  @ %bb.1: @ %if.then
+; CHECK-NEXT:    b bar
+; CHECK-NEXT:  .LBB1_2: @ %if.else
 ; CHECK-NEXT:    b bar
 entry:
   br i1 %i, label %if.then, label %if.else
 
 if.then:
   tail call void @bar() #0
   br label %if.end
 
 if.else:
   tail call void @bar() #0
   br label %if.end
 
 if.end:
   ret void
 }
 
 declare void @bar()
 
 attributes #0 = { nomerge }

llvm/test/CodeGen/LoongArch/nomerge.ll

if.end3:
   tail call void @bar() #0
   ret void
 }
 
 define void @foo_tail(i1 %i) nounwind {
 ; CHECK-LABEL: foo_tail:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    andi $a0, $a0, 1
+; CHECK-NEXT:    beqz $a0, .LBB1_2
+; CHECK-NEXT:  # %bb.1: # %if.then
+; CHECK-NEXT:    b %plt(bar)
+; CHECK-NEXT:  .LBB1_2: # %if.else
 ; CHECK-NEXT:    b %plt(bar)
 entry:
   br i1 %i, label %if.then, label %if.else
 
 if.then:
   tail call void @bar() #0
   br label %if.end
 

llvm/test/CodeGen/RISCV/nomerge.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
 ; RUN: llc < %s -mtriple=riscv64 -o - | FileCheck %s
 
 define void @foo(i32 %i) nounwind {
 ; CHECK-LABEL: foo:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    addi sp, sp, -16
 ; CHECK-NEXT:    sd ra, 8(sp) # 8-byte Folded Spill
 ; CHECK-NEXT:    sext.w a0, a0
 ; CHECK-NEXT:    li a1, 7
 ; CHECK-NEXT:    beq a0, a1, .LBB0_3
 ; CHECK-NEXT:  # %bb.1: # %entry
 ; CHECK-NEXT:    li a1, 5
 ; CHECK-NEXT:    bne a0, a1, .LBB0_4
 ; CHECK-NEXT:  # %bb.2: # %if.then
 ; CHECK-NEXT:    call bar@plt
if.then2:
   tail call void @bar() #0
   br label %if.end3
 
 if.end3:
   tail call void @bar() #0
   ret void
 }
 
-define void @foo_tail(i1 %i) nounwind {
+define void @foo_tail(i1 %i) nounwind{
 ; CHECK-LABEL: foo_tail:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    andi a0, a0, 1
+; CHECK-NEXT:    beqz a0, .LBB1_2
+; CHECK-NEXT:  # %bb.1: # %if.then
+; CHECK-NEXT:    tail bar@plt
+; CHECK-NEXT:  .LBB1_2: # %if.else
 ; CHECK-NEXT:    tail bar@plt
 entry:
   br i1 %i, label %if.then, label %if.else
 
 if.then:
   tail call void @bar() #0
   br label %if.end
 

llvm/test/CodeGen/SystemZ/nomerge.ll

if.end3:
   tail call void @bar() #0
   ret void
 }
 
 define void @foo_tail(i1 %i) nounwind {
 ; CHECK-LABEL: foo_tail:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    tmll %r2, 1
+; CHECK-NEXT:    jge bar@PLT
+; CHECK-NEXT:  .LBB1_1: # %if.then
 ; CHECK-NEXT:    jg bar@PLT
 entry:
   br i1 %i, label %if.then, label %if.else
 
 if.then:
   tail call void @bar() #0
   br label %if.end
 

llvm/test/CodeGen/X86/nomerge.ll

if.end3:
   tail call void @bar() #0
   ret void
 }
 
 define void @foo_tail(i1 %i) nounwind {
 ; CHECK-LABEL: foo_tail:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    testb $1, %dil
+; CHECK-NEXT:    je bar # TAILCALL
+; CHECK-NEXT:  # %bb.1: # %if.then
 ; CHECK-NEXT:    jmp bar # TAILCALL
 entry:
   br i1 %i, label %if.then, label %if.else
 
 if.then:
   tail call void @bar() #0
   br label %if.end