[Clang] Fix unnecessary extra return block emmited during function epilog after musttail call

[kiran-isaac]

closes #104770.

We clear the insert point after musttail call, and check for this when generating the function epilog.
We also check for a return point when emitting the return statement for a complex value

[llvmbot]

@llvm/pr-subscribers-clang-codegen

Author: Kiran (kiran-isaac)

Changes

closes #104770.

We clear the insert point after musttail call, and check for this when generating the function epilog.
We also check for a return point when emitting the return statement for a complex value

---

Full diff: https://github.com/llvm/llvm-project/pull/134282.diff

2 Files Affected:

• (modified) clang/lib/CodeGen/CGCall.cpp (+7-1)
• (modified) clang/lib/CodeGen/CGExprComplex.cpp (+4-1)

diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index b202255c3a15b..81a915ed7de22 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -3897,6 +3897,13 @@ void CodeGenFunction::EmitFunctionEpilog(const CGFunctionInfo &FI, return; } + // If there is no valid insert point, we won't emit a return. + // The insert point could be null if we have already emitted a return + // (e.g. if musttail) + if (!HaveInsertPoint()) { + return; + } + llvm::DebugLoc RetDbgLoc; llvm::Value *RV = nullptr; QualType RetTy = FI.getReturnType(); @@ -5990,7 +5997,6 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, else Builder.CreateRet(CI); Builder.ClearInsertionPoint(); - EnsureInsertPoint(); return GetUndefRValue(RetTy); } diff --git a/clang/lib/CodeGen/CGExprComplex.cpp b/clang/lib/CodeGen/CGExprComplex.cpp index f556594f4a9ec..a9b0a15c0383c 100644 --- a/clang/lib/CodeGen/CGExprComplex.cpp +++ b/clang/lib/CodeGen/CGExprComplex.cpp @@ -1491,7 +1491,10 @@ void CodeGenFunction::EmitComplexExprIntoLValue(const Expr *E, LValue dest, "Invalid complex expression to emit"); ComplexExprEmitter Emitter(*this); ComplexPairTy Val = Emitter.Visit(const_cast<Expr*>(E)); - Emitter.EmitStoreOfComplex(Val, dest, isInit); + // The insert point may be empty if we have just emmited a  + // musttail call. + if (HaveInsertPoint()) + Emitter.EmitStoreOfComplex(Val, dest, isInit); } /// EmitStoreOfComplex - Store a complex number into the specified l-value. 

[llvmbot]

@llvm/pr-subscribers-clang

Author: Kiran (kiran-isaac)

Changes

closes #104770.

We clear the insert point after musttail call, and check for this when generating the function epilog.
We also check for a return point when emitting the return statement for a complex value

---

Full diff: https://github.com/llvm/llvm-project/pull/134282.diff

2 Files Affected:

• (modified) clang/lib/CodeGen/CGCall.cpp (+7-1)
• (modified) clang/lib/CodeGen/CGExprComplex.cpp (+4-1)

diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index b202255c3a15b..81a915ed7de22 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -3897,6 +3897,13 @@ void CodeGenFunction::EmitFunctionEpilog(const CGFunctionInfo &FI, return; } + // If there is no valid insert point, we won't emit a return. + // The insert point could be null if we have already emitted a return + // (e.g. if musttail) + if (!HaveInsertPoint()) { + return; + } + llvm::DebugLoc RetDbgLoc; llvm::Value *RV = nullptr; QualType RetTy = FI.getReturnType(); @@ -5990,7 +5997,6 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, else Builder.CreateRet(CI); Builder.ClearInsertionPoint(); - EnsureInsertPoint(); return GetUndefRValue(RetTy); } diff --git a/clang/lib/CodeGen/CGExprComplex.cpp b/clang/lib/CodeGen/CGExprComplex.cpp index f556594f4a9ec..a9b0a15c0383c 100644 --- a/clang/lib/CodeGen/CGExprComplex.cpp +++ b/clang/lib/CodeGen/CGExprComplex.cpp @@ -1491,7 +1491,10 @@ void CodeGenFunction::EmitComplexExprIntoLValue(const Expr *E, LValue dest, "Invalid complex expression to emit"); ComplexExprEmitter Emitter(*this); ComplexPairTy Val = Emitter.Visit(const_cast<Expr*>(E)); - Emitter.EmitStoreOfComplex(Val, dest, isInit); + // The insert point may be empty if we have just emmited a  + // musttail call. + if (HaveInsertPoint()) + Emitter.EmitStoreOfComplex(Val, dest, isInit); } /// EmitStoreOfComplex - Store a complex number into the specified l-value. 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[efriedma-quic]

There's a comment in CodeGenFunction::EmitReturnBlock that says:

 // FIXME: We are at an unreachable point, there is no reason to emit the block // unless it has uses. However, we still need a place to put the debug // region.end for now. 

Is that out of date?

---

Note that for most functions, we actually emit the return block, then erase it if it's unused. (See the end of CodeGenFunction::FinishFunction).

[efriedma-quic]

Suggested change

	// The insert point may be empty if we have just emmited a
	// The insert point may be empty if we have just emitted a

[efriedma-quic]

The placement of this check doesn't seem right; probably it should be before we start constructing instructions?

[rnk]

This patch could use tests in general, and I wanted to add, you should be able to cover this case with a musttail call in a void-returning function, which should exercise one of the paths above.