[AsmWriter] Do not write a comma when varargs is the only argument

[caojoshua]

Summary

Fixes https://github.com/llvm/llvm-project/issues/56544

AsmWriter always writes ", ..." when a tail call has a varargs argument. This patch only writes the ", " when there is an argument before the varargs argument.

I did not write a dedicated test this for this change, but I modified an existing test that will test for a regression.

[avogelsgesang]

LGTM, with small nit:

There is a typo in the commit description

I did not write a dedicated this for this change

should be

I did not write a dedicated test for this change

Please fix before merging

[caojoshua]

In D137893#3923303, @avogelsgesang wrote:

LGTM, with small nit:

There is a typo in the commit description

I did not write a dedicated this for this change

should be

I did not write a dedicated test for this change

Please fix before merging

Thank you. I fixed it.

Can I get help pushing this? This is my first contribution, and I do not have permissions.

[avogelsgesang]

Can you provide me a git patch, including the meta data (email address, ...)?
Not sure how to get this from Differential, I am rather inexperienced with it myself...

[caojoshua]

I have my git commit in in my github fork

commit 50a55613441151671f6a49982acea9fb043f7d5e
Author: Joshua Cao <[e-mail]>
Date:   Sun Nov 13 22:24:15 2022 -0500

    [AsmWriter] Do not write a comma when varargs is the only argument
    
    Fixes https://github.com/llvm/llvm-project/issues/56544
    
    AsmWriter always writes ", ..." when a tail call has a varargs argument. This patch only writes the ", " when there is an argument before the varargs argument.
    
    I did not write a dedicated test this for this change, but I modified an existing test that will test for a regression.
    
    Reviewed By: avogelsgesang
    
    Differential Revision: https://reviews.llvm.org/D137893

[avogelsgesang]

Pushed upstream. Thank you for the fix!

llvm/lib/IR/AsmWriter.cpp

if (PAL.hasRetAttrs())
       Out << ' ' << PAL.getAsString(AttributeList::ReturnIndex);
 
     // Only print addrspace(N) if necessary:
     maybePrintCallAddrSpace(Operand, &I, Out);
 
     // If possible, print out the short form of the call instruction.  We can
     // only do this if the first argument is a pointer to a nonvararg function,
     // and if the return type is not a pointer to a function.
-    //
     Out << ' ';
     TypePrinter.print(FTy->isVarArg() ? FTy : RetTy, Out);
     Out << ' ';
     writeOperand(Operand, false);
     Out << '(';
     for (unsigned op = 0, Eop = CI->arg_size(); op < Eop; ++op) {
       if (op > 0)
         Out << ", ";
       writeParamOperand(CI->getArgOperand(op), PAL.getParamAttrs(op));
     }
 
     // Emit an ellipsis if this is a musttail call in a vararg function.  This
     // is only to aid readability, musttail calls forward varargs by default.
     if (CI->isMustTailCall() && CI->getParent() &&
         CI->getParent()->getParent() &&
-        CI->getParent()->getParent()->isVarArg())
-      Out << ", ...";
+        CI->getParent()->getParent()->isVarArg()) {
+      if (CI->arg_size() > 0)
+        Out << ", ";
+      Out << "...";
+    }
 
     Out << ')';
     if (PAL.hasFnAttrs())
       Out << " #" << Machine.getAttributeGroupSlot(PAL.getFnAttrs());
 
     writeOperandBundles(CI);
   } else if (const InvokeInst *II = dyn_cast<InvokeInst>(&I)) {
     Operand = II->getCalledOperand();

llvm/test/Transforms/Inline/inline-brunch-funnel.ll

 ; Test that inliner skips @llvm.icall.branch.funnel
 ; RUN: opt < %s -inline -S | FileCheck %s
 
 target datalayout = "e-p:64:64"
 target triple = "x86_64-unknown-linux-gnu"
 
 declare void @llvm.icall.branch.funnel(...)
 
 ; CHECK-LABEL: define void @fn_musttail(
 define void @fn_musttail() {
   call void (...) @bf_musttail()
-  ; CHECK: call void (...) @bf_musttail(
+  ; CHECK: call void (...) @bf_musttail()
   ret void
 }
 
 ; CHECK-LABEL: define internal void @bf_musttail(
 define internal void @bf_musttail(...) {
   musttail call void (...) @llvm.icall.branch.funnel(...)
-  ; CHECK: musttail call void (...) @llvm.icall.branch.funnel(
+  ; CHECK: musttail call void (...) @llvm.icall.branch.funnel(...)
   ret void
 }
 
 ; CHECK-LABEL: define void @fn_musttail_always(
 define void @fn_musttail_always() {
   call void (...) @bf_musttail_always()
-  ; CHECK: call void (...) @bf_musttail_always(
+  ; CHECK: call void (...) @bf_musttail_always()
   ret void
 }
 
 ; CHECK-LABEL: define internal void @bf_musttail_always(
 define internal void @bf_musttail_always(...) alwaysinline {
   musttail call void (...) @llvm.icall.branch.funnel(...)
-  ; CHECK: musttail call void (...) @llvm.icall.branch.funnel(
+  ; CHECK: musttail call void (...) @llvm.icall.branch.funnel(...)
   ret void
 }