https://github.com/chenzheng1030 updated https://github.com/llvm/llvm-project/pull/93267
>From 6047403b41bdeef8cd16294f0538150b8c58f6ea Mon Sep 17 00:00:00 2001 From: Chen Zheng <czhen...@cn.ibm.com> Date: Thu, 23 May 2024 22:59:48 -0400 Subject: [PATCH 1/3] [PowerPC] Diagnose musttail instead of crash inside backend musttail does not often possible to generate on PPC targets as when calling to a function defined in another model, PPC needs to restore the TOC pointer. To restore the TOC pointer, compiler needs to emit a nop after the function call to let linker generate codes to restore TOC pointer. Tail call does not generate expected call sequence for this case. To avoid the crash inside the compiler backend, a diagnosis is added in the frontend and in the backend, PPC will change the musttail to tail. Fixes #63214 --- .../clang/Basic/DiagnosticSemaKinds.td | 5 +++ clang/lib/CodeGen/CGCall.cpp | 10 +++++- clang/test/SemaCXX/attr-musttail-ppc.cpp | 12 +++++++ llvm/lib/Target/PowerPC/PPCISelLowering.cpp | 15 +++++++-- llvm/test/CodeGen/PowerPC/musttail-call.ll | 32 +++++++++++++++++++ 5 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 clang/test/SemaCXX/attr-musttail-ppc.cpp create mode 100644 llvm/test/CodeGen/PowerPC/musttail-call.ll diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 25a87078a5709..c75f40a4a42f4 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3841,6 +3841,11 @@ def note_cannot_use_trivial_abi_reason : Note< "it is polymorphic|" "it has a base of a non-trivial class type|it has a virtual base|" "it has a __weak field|it has a field of a non-trivial class type}1">; +def warn_ppc_musttail_maybe_ignored: Warning< + "'musttail' attribute may be ignored on ppc targets">, + InGroup<IgnoredAttributes>; +def err_aix_musttail_unsupported: Error< + "'musttail' attribute is not supported on AIX">; // Availability attribute def warn_availability_unknown_platform : Warning< diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index 2b301130ef7b7..dae3d3287f683 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -26,6 +26,7 @@ #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/Basic/CodeGenOptions.h" +#include "clang/Basic/DiagnosticSema.h" #include "clang/Basic/TargetInfo.h" #include "clang/CodeGen/CGFunctionInfo.h" #include "clang/CodeGen/SwiftCallingConv.h" @@ -5751,8 +5752,15 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, if (llvm::CallInst *Call = dyn_cast<llvm::CallInst>(CI)) { if (TargetDecl && TargetDecl->hasAttr<NotTailCalledAttr>()) Call->setTailCallKind(llvm::CallInst::TCK_NoTail); - else if (IsMustTail) + else if (IsMustTail) { + if (getTarget().getTriple().isPPC()) { + if (getTarget().getTriple().isOSAIX()) + CGM.getDiags().Report(Loc, diag::err_aix_musttail_unsupported); + else + CGM.getDiags().Report(Loc, diag::warn_ppc_musttail_maybe_ignored); + } Call->setTailCallKind(llvm::CallInst::TCK_MustTail); + } } // Add metadata for calls to MSAllocator functions diff --git a/clang/test/SemaCXX/attr-musttail-ppc.cpp b/clang/test/SemaCXX/attr-musttail-ppc.cpp new file mode 100644 index 0000000000000..72b61adf7560b --- /dev/null +++ b/clang/test/SemaCXX/attr-musttail-ppc.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 %s -triple powerpc64-ibm-aix-xcoff -o /dev/null -emit-llvm -verify=aix +// RUN: %clang_cc1 %s -triple powerpc-ibm-aix-xcoff -o /dev/null -emit-llvm -verify=aix +// RUN: %clang_cc1 %s -triple powerpc64-unknown-linux-gnu -o /dev/null -emit-llvm -verify=linux +// RUN: %clang_cc1 %s -triple powerpc-unknown-linux-gnu -o /dev/null -emit-llvm -verify=linux +// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -o /dev/null -emit-llvm -verify=linux + +int Func(); +int Func1() { + // linux-warning@+2 {{'musttail' attribute may be ignored on ppc targets}} + // aix-error@+1 {{'musttail' attribute is not supported on AIX}} + [[clang::musttail]] return Func(); +} diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp index 9e56b8522fa63..95d82f1db94d1 100644 --- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp +++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp @@ -146,6 +146,10 @@ static cl::opt<unsigned> PPCAIXTLSModelOptUseIEForLDLimit( cl::desc("Set inclusive limit count of TLS local-dynamic access(es) in a " "function to use initial-exec")); +static cl::opt<bool> AbortOnImpossibleMusttailCall( + "ppc-abort-on-impossible-musttailcall", cl::init(false), cl::Hidden, + cl::desc("Abort if any call marked as musttail is impossible.")); + STATISTIC(NumTailCalls, "Number of tail calls"); STATISTIC(NumSiblingCalls, "Number of sibling calls"); STATISTIC(ShufflesHandledWithVPERM, @@ -5945,9 +5949,14 @@ PPCTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI, } } - if (!isTailCall && CB && CB->isMustTailCall()) - report_fatal_error("failed to perform tail call elimination on a call " - "site marked musttail"); + if (!isTailCall && CB && CB->isMustTailCall()) { + if (AbortOnImpossibleMusttailCall) + report_fatal_error("failed to perform tail call elimination on a call " + "site marked musttail"); + else + cast<CallInst>(const_cast<CallBase *>(CB)) + ->setTailCallKind(llvm::CallInst::TCK_Tail); + } // When long calls (i.e. indirect calls) are always used, calls are always // made via function pointer. If we have a function name, first translate it diff --git a/llvm/test/CodeGen/PowerPC/musttail-call.ll b/llvm/test/CodeGen/PowerPC/musttail-call.ll new file mode 100644 index 0000000000000..c4c283f5e1f94 --- /dev/null +++ b/llvm/test/CodeGen/PowerPC/musttail-call.ll @@ -0,0 +1,32 @@ +; RUN: not --crash llc -mtriple=powerpc64-ibm-aix-xcoff %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=true | \ +; RUN: FileCheck %s --check-prefix=CRASH +; RUN: not --crash llc -mtriple=powerpc-ibm-aix-xcoff %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=true | \ +; RUN: FileCheck %s --check-prefix=CRASH +; RUN: not --crash llc -mtriple=powerpc64-unknown-linux-gnu %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=true | \ +; RUN: FileCheck %s --check-prefix=CRASH +; RUN: not --crash llc -mtriple=powerpc-unknown-linux-gnu %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=true | \ +; RUN: FileCheck %s --check-prefix=CRASH +; RUN: not --crash llc -mtriple=powerpc64le-unknown-linux-gnu %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=true | \ +; RUN: FileCheck %s --check-prefix=CRASH +; RUN: llc -mtriple=powerpc64-ibm-aix-xcoff %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=false | \ +; RUN: FileCheck %s --check-prefix=NOCRASH +; RUN: llc -mtriple=powerpc-ibm-aix-xcoff %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=false | \ +; RUN: FileCheck %s --check-prefix=NOCRASH +; RUN: llc -mtriple=powerpc64-unknown-linux-gnu %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=false | \ +; RUN: FileCheck %s --check-prefix=NOCRASH +; RUN: llc -mtriple=powerpc-unknown-linux-gnu %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=false | \ +; RUN: FileCheck %s --check-prefix=NOCRASH +; RUN: llc -mtriple=powerpc64le-unknown-linux-gnu %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=false | \ +; RUN: FileCheck %s --check-prefix=NOCRASH + +; CRASH: LLVM ERROR: failed to perform tail call elimination on a call site marked musttail +; NOCRASH-NOT: LLVM ERROR: failed to perform tail call elimination on a call site marked musttail +; NOCRASH-LABEL: caller +; NOCRASH: bl {{callee|.callee}} + + +declare i32 @callee() +define i32 @caller() { + %res = musttail call i32 @callee() + ret i32 %res +} >From 1fdf5ef7e29c7cdf76e4ae95562cb0b0c1942324 Mon Sep 17 00:00:00 2001 From: Chen Zheng <czhen...@cn.ibm.com> Date: Wed, 29 May 2024 03:21:20 -0400 Subject: [PATCH 2/3] address comments --- .../clang/Basic/DiagnosticSemaKinds.td | 9 ++-- clang/lib/Basic/Targets/PPC.cpp | 3 ++ clang/lib/Basic/Targets/PPC.h | 1 + clang/lib/CodeGen/CGCall.cpp | 10 +---- clang/lib/Sema/SemaStmt.cpp | 14 +++++++ clang/test/SemaCXX/attr-musttail-ppc.cpp | 42 +++++++++++++++---- llvm/lib/Target/PowerPC/PPCISelLowering.cpp | 15 ++----- llvm/test/CodeGen/PowerPC/musttail-call.ll | 32 -------------- 8 files changed, 61 insertions(+), 65 deletions(-) delete mode 100644 llvm/test/CodeGen/PowerPC/musttail-call.ll diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index c75f40a4a42f4..1797f59a0e077 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3841,9 +3841,12 @@ def note_cannot_use_trivial_abi_reason : Note< "it is polymorphic|" "it has a base of a non-trivial class type|it has a virtual base|" "it has a __weak field|it has a field of a non-trivial class type}1">; -def warn_ppc_musttail_maybe_ignored: Warning< - "'musttail' attribute may be ignored on ppc targets">, - InGroup<IgnoredAttributes>; +def err_ppc_impossible_musttail: Error< + "'musttail' attribute for this call is impossible because %select{" + "long calls can not be tail called|" + "indirect calls can not be tail called|" + "external calls can not be tail called}0" + >; def err_aix_musttail_unsupported: Error< "'musttail' attribute is not supported on AIX">; diff --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp index a1e5f20f7dbe2..9ba0d108e3281 100644 --- a/clang/lib/Basic/Targets/PPC.cpp +++ b/clang/lib/Basic/Targets/PPC.cpp @@ -93,6 +93,8 @@ bool PPCTargetInfo::handleTargetFeatures(std::vector<std::string> &Features, HasQuadwordAtomics = true; } else if (Feature == "+aix-shared-lib-tls-model-opt") { HasAIXShLibTLSModelOpt = true; + } else if (Feature == "+longcall") { + UseLongCalls = true; } // TODO: Finish this list and add an assert that we've handled them // all. @@ -728,6 +730,7 @@ bool PPCTargetInfo::hasFeature(StringRef Feature) const { .Case("isa-v31-instructions", IsISA3_1) .Case("quadword-atomics", HasQuadwordAtomics) .Case("aix-shared-lib-tls-model-opt", HasAIXShLibTLSModelOpt) + .Case("longcall", UseLongCalls) .Default(false); } diff --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h index fc23c30c68523..316174beca8a8 100644 --- a/clang/lib/Basic/Targets/PPC.h +++ b/clang/lib/Basic/Targets/PPC.h @@ -82,6 +82,7 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetInfo : public TargetInfo { bool IsISA3_1 = false; bool HasQuadwordAtomics = false; bool HasAIXShLibTLSModelOpt = false; + bool UseLongCalls = false; protected: std::string ABI; diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index dae3d3287f683..2b301130ef7b7 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -26,7 +26,6 @@ #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/Basic/CodeGenOptions.h" -#include "clang/Basic/DiagnosticSema.h" #include "clang/Basic/TargetInfo.h" #include "clang/CodeGen/CGFunctionInfo.h" #include "clang/CodeGen/SwiftCallingConv.h" @@ -5752,15 +5751,8 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, if (llvm::CallInst *Call = dyn_cast<llvm::CallInst>(CI)) { if (TargetDecl && TargetDecl->hasAttr<NotTailCalledAttr>()) Call->setTailCallKind(llvm::CallInst::TCK_NoTail); - else if (IsMustTail) { - if (getTarget().getTriple().isPPC()) { - if (getTarget().getTriple().isOSAIX()) - CGM.getDiags().Report(Loc, diag::err_aix_musttail_unsupported); - else - CGM.getDiags().Report(Loc, diag::warn_ppc_musttail_maybe_ignored); - } + else if (IsMustTail) Call->setTailCallKind(llvm::CallInst::TCK_MustTail); - } } // Add metadata for calls to MSAllocator functions diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 1bb86385333ef..8a96c09334bcb 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -743,6 +743,20 @@ bool Sema::checkMustTailAttr(const Stmt *St, const Attr &MTA) { CallerType.Func = CallerDecl->getType()->getAs<FunctionProtoType>(); } + if (Context.getTargetInfo().getTriple().isPPC()) { + if (Context.getTargetInfo().getTriple().isOSAIX()) + return Diag(St->getBeginLoc(), diag::err_aix_musttail_unsupported); + else if (!Context.getTargetInfo().hasFeature("pcrelative-memops")) { + if (Context.getTargetInfo().hasFeature("longcall")) + return Diag(St->getBeginLoc(), diag::err_ppc_impossible_musttail) << 0; + else if (!CE->getDirectCallee()) + return Diag(St->getBeginLoc(), diag::err_ppc_impossible_musttail) << 1; + else if (isa_and_nonnull<FunctionDecl>(CE->getCalleeDecl()) && + !cast<FunctionDecl>(CE->getCalleeDecl())->isDefined()) + return Diag(St->getBeginLoc(), diag::err_ppc_impossible_musttail) << 2; + } + } + const Expr *CalleeExpr = CE->getCallee()->IgnoreParens(); const auto *CalleeBinOp = dyn_cast<BinaryOperator>(CalleeExpr); SourceLocation CalleeLoc = CE->getCalleeDecl() diff --git a/clang/test/SemaCXX/attr-musttail-ppc.cpp b/clang/test/SemaCXX/attr-musttail-ppc.cpp index 72b61adf7560b..3066dc5394efd 100644 --- a/clang/test/SemaCXX/attr-musttail-ppc.cpp +++ b/clang/test/SemaCXX/attr-musttail-ppc.cpp @@ -1,12 +1,36 @@ -// RUN: %clang_cc1 %s -triple powerpc64-ibm-aix-xcoff -o /dev/null -emit-llvm -verify=aix -// RUN: %clang_cc1 %s -triple powerpc-ibm-aix-xcoff -o /dev/null -emit-llvm -verify=aix -// RUN: %clang_cc1 %s -triple powerpc64-unknown-linux-gnu -o /dev/null -emit-llvm -verify=linux -// RUN: %clang_cc1 %s -triple powerpc-unknown-linux-gnu -o /dev/null -emit-llvm -verify=linux -// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -o /dev/null -emit-llvm -verify=linux +// RUN: %clang_cc1 %s -triple powerpc64-ibm-aix-xcoff -fsyntax-only -verify=aix +// RUN: %clang_cc1 %s -triple powerpc-ibm-aix-xcoff -fsyntax-only -verify=aix +// RUN: %clang_cc1 %s -triple powerpc64-unknown-linux-gnu -fsyntax-only -verify=linux +// RUN: %clang_cc1 %s -triple powerpc-unknown-linux-gnu -fsyntax-only -verify=linux +// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -fsyntax-only -verify=linux +// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -target-feature +pcrelative-memops -fsyntax-only -verify=good +// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -target-feature +longcall -fsyntax-only -verify=longcall +// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -target-feature +pcrelative-memops -target-feature +longcall -fsyntax-only -verify=good -int Func(); -int Func1() { - // linux-warning@+2 {{'musttail' attribute may be ignored on ppc targets}} +int good_callee() { + return 0; +} +int good_caller() { + // good-no-diagnostics + // longcall-error@+2 {{'musttail' attribute for this call is impossible because long calls can not be tail called}} + // aix-error@+1 {{'musttail' attribute is not supported on AIX}} + [[clang::musttail]] return good_callee(); +} + +int func(); +int external_call() { + // good-no-diagnostics + // longcall-error@+3 {{'musttail' attribute for this call is impossible because long calls can not be tail called}} + // linux-error@+2 {{'musttail' attribute for this call is impossible because external calls can not be tail called}} + // aix-error@+1 {{'musttail' attribute is not supported on AIX}} + [[clang::musttail]] return func(); +} + +void indirect_call(int r) { + auto Fn = (void (*)(int))1; + // good-no-diagnostics + // longcall-error@+3 {{'musttail' attribute for this call is impossible because long calls can not be tail called}} + // linux-error@+2 {{'musttail' attribute for this call is impossible because indirect calls can not be tail called}} // aix-error@+1 {{'musttail' attribute is not supported on AIX}} - [[clang::musttail]] return Func(); + [[clang::musttail]] return Fn(r); } diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp index 95d82f1db94d1..9e56b8522fa63 100644 --- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp +++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp @@ -146,10 +146,6 @@ static cl::opt<unsigned> PPCAIXTLSModelOptUseIEForLDLimit( cl::desc("Set inclusive limit count of TLS local-dynamic access(es) in a " "function to use initial-exec")); -static cl::opt<bool> AbortOnImpossibleMusttailCall( - "ppc-abort-on-impossible-musttailcall", cl::init(false), cl::Hidden, - cl::desc("Abort if any call marked as musttail is impossible.")); - STATISTIC(NumTailCalls, "Number of tail calls"); STATISTIC(NumSiblingCalls, "Number of sibling calls"); STATISTIC(ShufflesHandledWithVPERM, @@ -5949,14 +5945,9 @@ PPCTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI, } } - if (!isTailCall && CB && CB->isMustTailCall()) { - if (AbortOnImpossibleMusttailCall) - report_fatal_error("failed to perform tail call elimination on a call " - "site marked musttail"); - else - cast<CallInst>(const_cast<CallBase *>(CB)) - ->setTailCallKind(llvm::CallInst::TCK_Tail); - } + if (!isTailCall && CB && CB->isMustTailCall()) + report_fatal_error("failed to perform tail call elimination on a call " + "site marked musttail"); // When long calls (i.e. indirect calls) are always used, calls are always // made via function pointer. If we have a function name, first translate it diff --git a/llvm/test/CodeGen/PowerPC/musttail-call.ll b/llvm/test/CodeGen/PowerPC/musttail-call.ll deleted file mode 100644 index c4c283f5e1f94..0000000000000 --- a/llvm/test/CodeGen/PowerPC/musttail-call.ll +++ /dev/null @@ -1,32 +0,0 @@ -; RUN: not --crash llc -mtriple=powerpc64-ibm-aix-xcoff %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=true | \ -; RUN: FileCheck %s --check-prefix=CRASH -; RUN: not --crash llc -mtriple=powerpc-ibm-aix-xcoff %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=true | \ -; RUN: FileCheck %s --check-prefix=CRASH -; RUN: not --crash llc -mtriple=powerpc64-unknown-linux-gnu %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=true | \ -; RUN: FileCheck %s --check-prefix=CRASH -; RUN: not --crash llc -mtriple=powerpc-unknown-linux-gnu %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=true | \ -; RUN: FileCheck %s --check-prefix=CRASH -; RUN: not --crash llc -mtriple=powerpc64le-unknown-linux-gnu %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=true | \ -; RUN: FileCheck %s --check-prefix=CRASH -; RUN: llc -mtriple=powerpc64-ibm-aix-xcoff %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=false | \ -; RUN: FileCheck %s --check-prefix=NOCRASH -; RUN: llc -mtriple=powerpc-ibm-aix-xcoff %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=false | \ -; RUN: FileCheck %s --check-prefix=NOCRASH -; RUN: llc -mtriple=powerpc64-unknown-linux-gnu %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=false | \ -; RUN: FileCheck %s --check-prefix=NOCRASH -; RUN: llc -mtriple=powerpc-unknown-linux-gnu %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=false | \ -; RUN: FileCheck %s --check-prefix=NOCRASH -; RUN: llc -mtriple=powerpc64le-unknown-linux-gnu %s -o - 2>&1 -ppc-abort-on-impossible-musttailcall=false | \ -; RUN: FileCheck %s --check-prefix=NOCRASH - -; CRASH: LLVM ERROR: failed to perform tail call elimination on a call site marked musttail -; NOCRASH-NOT: LLVM ERROR: failed to perform tail call elimination on a call site marked musttail -; NOCRASH-LABEL: caller -; NOCRASH: bl {{callee|.callee}} - - -declare i32 @callee() -define i32 @caller() { - %res = musttail call i32 @callee() - ret i32 %res -} >From e373eeab65b25ef84968e4b4c64be779451f8615 Mon Sep 17 00:00:00 2001 From: Chen Zheng <czhen...@cn.ibm.com> Date: Mon, 24 Jun 2024 01:30:51 -0400 Subject: [PATCH 3/3] address comments --- .../clang/Basic/DiagnosticCommonKinds.td | 9 +++++ .../clang/Basic/DiagnosticSemaKinds.td | 8 ----- clang/lib/CodeGen/CGCall.cpp | 23 +++++++++++- clang/lib/CodeGen/CodeGenModule.cpp | 8 +++++ clang/lib/CodeGen/CodeGenModule.h | 13 +++++++ clang/lib/Sema/SemaStmt.cpp | 14 -------- .../PowerPC/musttail-forward-declaration.c | 11 ++++++ .../CodeGen/PowerPC/musttail-indirect.cpp | 8 +++++ .../test/CodeGen/PowerPC/musttail-undefined.c | 10 ++++++ clang/test/CodeGen/PowerPC/musttail-weak.c | 13 +++++++ clang/test/CodeGen/PowerPC/musttail.c | 20 +++++++++++ clang/test/SemaCXX/attr-musttail-ppc.cpp | 36 ------------------- 12 files changed, 114 insertions(+), 59 deletions(-) create mode 100644 clang/test/CodeGen/PowerPC/musttail-forward-declaration.c create mode 100644 clang/test/CodeGen/PowerPC/musttail-indirect.cpp create mode 100644 clang/test/CodeGen/PowerPC/musttail-undefined.c create mode 100644 clang/test/CodeGen/PowerPC/musttail-weak.c create mode 100644 clang/test/CodeGen/PowerPC/musttail.c delete mode 100644 clang/test/SemaCXX/attr-musttail-ppc.cpp diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td b/clang/include/clang/Basic/DiagnosticCommonKinds.td index de758cbe679dc..33b1d58bb5b09 100644 --- a/clang/include/clang/Basic/DiagnosticCommonKinds.td +++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td @@ -367,6 +367,15 @@ def warn_target_unrecognized_env : Warning< def err_target_unsupported_abi_with_fpu : Error< "'%0' ABI is not supported with FPU">; +def err_ppc_impossible_musttail: Error< + "'musttail' attribute for this call is impossible because %select{" + "long calls can not be tail called on PPC|" + "indirect calls can not be tail called on PPC|" + "external calls can not be tail called on PPC}0" + >; +def err_aix_musttail_unsupported: Error< + "'musttail' attribute is not supported on AIX">; + // Source manager def err_cannot_open_file : Error<"cannot open file '%0': %1">, DefaultFatal; def err_file_modified : Error< diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 1797f59a0e077..25a87078a5709 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3841,14 +3841,6 @@ def note_cannot_use_trivial_abi_reason : Note< "it is polymorphic|" "it has a base of a non-trivial class type|it has a virtual base|" "it has a __weak field|it has a field of a non-trivial class type}1">; -def err_ppc_impossible_musttail: Error< - "'musttail' attribute for this call is impossible because %select{" - "long calls can not be tail called|" - "indirect calls can not be tail called|" - "external calls can not be tail called}0" - >; -def err_aix_musttail_unsupported: Error< - "'musttail' attribute is not supported on AIX">; // Availability attribute def warn_availability_unknown_platform : Warning< diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index 2b301130ef7b7..8ea802e1b5f5a 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -5751,8 +5751,29 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, if (llvm::CallInst *Call = dyn_cast<llvm::CallInst>(CI)) { if (TargetDecl && TargetDecl->hasAttr<NotTailCalledAttr>()) Call->setTailCallKind(llvm::CallInst::TCK_NoTail); - else if (IsMustTail) + else if (IsMustTail) { + if (getTarget().getTriple().isPPC()) { + if (getTarget().getTriple().isOSAIX()) + CGM.getDiags().Report(Loc, diag::err_aix_musttail_unsupported); + else if (!getTarget().hasFeature("pcrelative-memops")) { + if (getTarget().hasFeature("longcall")) + CGM.getDiags().Report(Loc, diag::err_ppc_impossible_musttail) << 0; + else if (Call->isIndirectCall()) + CGM.getDiags().Report(Loc, diag::err_ppc_impossible_musttail) << 1; + else if (isa_and_nonnull<FunctionDecl>(TargetDecl) && + cast<FunctionDecl>(TargetDecl)->isWeak()) + CGM.getDiags().Report(Loc, diag::err_ppc_impossible_musttail) << 2; + else if (isa_and_nonnull<FunctionDecl>(TargetDecl) && + !cast<FunctionDecl>(TargetDecl)->isDefined()) + // The undefined callee may be a forward declaration. Without + // knowning all symbols in the module, we won't know the symbol is + // defined or not. Collect all these symbols for later diagnosing. + CGM.addUndefinedGlobalForTailCall( + {cast<FunctionDecl>(TargetDecl), Loc}); + } + } Call->setTailCallKind(llvm::CallInst::TCK_MustTail); + } } // Add metadata for calls to MSAllocator functions diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index dd4a665ebc78b..79d6e98dd6950 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -1393,6 +1393,14 @@ void CodeGenModule::Release() { // that might affect the DLL storage class or the visibility, and // before anything that might act on these. setVisibilityFromDLLStorageClass(LangOpts, getModule()); + + // Check the tail call symbols are truly undefined. + if (getTriple().isPPC() && !MustTailCallUndefinedGlobals.empty()) { + for (auto &I : MustTailCallUndefinedGlobals) { + if (!I.first->isDefined()) + getDiags().Report(I.second, diag::err_ppc_impossible_musttail) << 2; + } + } } void CodeGenModule::EmitOpenCLMetadata() { diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 9913304757caa..eef40f89d72c9 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -485,6 +485,14 @@ class CodeGenModule : public CodeGenTypeCache { typedef std::pair<OrderGlobalInitsOrStermFinalizers, llvm::Function *> GlobalInitData; + // When a tail call is performed on an "undefined" symbol, on PPC without pc + // relative feature, the tail call is not allowed. In "EmitCall" for such + // tail calls, the "undefined" symbols may be forward declarations, their + // definitions are provided in the module but after the callsites. For such + // tail calls, diagnose message should be not emitted. + llvm::SmallSetVector<std::pair<const FunctionDecl *, SourceLocation>, 4> + MustTailCallUndefinedGlobals; + struct GlobalInitPriorityCmp { bool operator()(const GlobalInitData &LHS, const GlobalInitData &RHS) const { @@ -1618,6 +1626,11 @@ class CodeGenModule : public CodeGenTypeCache { return getTriple().isSPIRVLogical(); } + void addUndefinedGlobalForTailCall( + std::pair<const FunctionDecl *, SourceLocation> Global) { + MustTailCallUndefinedGlobals.insert(Global); + } + private: bool shouldDropDLLAttribute(const Decl *D, const llvm::GlobalValue *GV) const; diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 8a96c09334bcb..1bb86385333ef 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -743,20 +743,6 @@ bool Sema::checkMustTailAttr(const Stmt *St, const Attr &MTA) { CallerType.Func = CallerDecl->getType()->getAs<FunctionProtoType>(); } - if (Context.getTargetInfo().getTriple().isPPC()) { - if (Context.getTargetInfo().getTriple().isOSAIX()) - return Diag(St->getBeginLoc(), diag::err_aix_musttail_unsupported); - else if (!Context.getTargetInfo().hasFeature("pcrelative-memops")) { - if (Context.getTargetInfo().hasFeature("longcall")) - return Diag(St->getBeginLoc(), diag::err_ppc_impossible_musttail) << 0; - else if (!CE->getDirectCallee()) - return Diag(St->getBeginLoc(), diag::err_ppc_impossible_musttail) << 1; - else if (isa_and_nonnull<FunctionDecl>(CE->getCalleeDecl()) && - !cast<FunctionDecl>(CE->getCalleeDecl())->isDefined()) - return Diag(St->getBeginLoc(), diag::err_ppc_impossible_musttail) << 2; - } - } - const Expr *CalleeExpr = CE->getCallee()->IgnoreParens(); const auto *CalleeBinOp = dyn_cast<BinaryOperator>(CalleeExpr); SourceLocation CalleeLoc = CE->getCalleeDecl() diff --git a/clang/test/CodeGen/PowerPC/musttail-forward-declaration.c b/clang/test/CodeGen/PowerPC/musttail-forward-declaration.c new file mode 100644 index 0000000000000..061a7a8c2da9d --- /dev/null +++ b/clang/test/CodeGen/PowerPC/musttail-forward-declaration.c @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -o /dev/null -emit-llvm -verify=good +// RUN: %clang_cc1 %s -triple powerpc64-unknown-linux-gnu -o /dev/null -emit-llvm -verify=good + +int func2(int i); +int external_call2(int i) { + // good-no-diagnostics + [[clang::musttail]] return func2(i); +} +int func2(int i) { + return 0; +} diff --git a/clang/test/CodeGen/PowerPC/musttail-indirect.cpp b/clang/test/CodeGen/PowerPC/musttail-indirect.cpp new file mode 100644 index 0000000000000..3f495002606d4 --- /dev/null +++ b/clang/test/CodeGen/PowerPC/musttail-indirect.cpp @@ -0,0 +1,8 @@ +// RUN: %clang_cc1 %s -triple powerpc64-unknown-linux-gnu -o /dev/null -emit-llvm -verify +// RUN: %clang_cc1 %s -triple powerpc-unknown-linux-gnu -o /dev/null -emit-llvm -verify + +void name(int *params) { + auto fn = (void (*)(int *))1; + // expected-error@+1 {{'musttail' attribute for this call is impossible because indirect calls can not be tail called on PPC}} + [[clang::musttail]] return fn(params); +} diff --git a/clang/test/CodeGen/PowerPC/musttail-undefined.c b/clang/test/CodeGen/PowerPC/musttail-undefined.c new file mode 100644 index 0000000000000..f2259adb01848 --- /dev/null +++ b/clang/test/CodeGen/PowerPC/musttail-undefined.c @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 %s -triple powerpc64-unknown-linux-gnu -o /dev/null -emit-llvm -verify +// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -o /dev/null -emit-llvm -verify + +int foo(int x); + +int bar(int x) +{ + // expected-error@+1 {{'musttail' attribute for this call is impossible because external calls can not be tail called on PPC}} + [[clang::musttail]] return foo(x); +} diff --git a/clang/test/CodeGen/PowerPC/musttail-weak.c b/clang/test/CodeGen/PowerPC/musttail-weak.c new file mode 100644 index 0000000000000..dccc7a4d8cdd2 --- /dev/null +++ b/clang/test/CodeGen/PowerPC/musttail-weak.c @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 %s -triple powerpc64-ibm-aix-xcoff -o /dev/null -emit-llvm -verify=aix +// RUN: %clang_cc1 %s -triple powerpc-ibm-aix-xcoff -o /dev/null -emit-llvm -verify=aix +// RUN: %clang_cc1 %s -triple powerpc64-unknown-linux-gnu -o /dev/null -emit-llvm -verify=linux +// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -o /dev/null -emit-llvm -verify=linux + +__attribute__((weak)) int func2(int i) { + return 0; +} +int external_call2(int i) { + // linux-error@+2 {{'musttail' attribute for this call is impossible because external calls can not be tail called on PPC}} + // aix-error@+1 {{'musttail' attribute is not supported on AIX}} + [[clang::musttail]] return func2(i); +} diff --git a/clang/test/CodeGen/PowerPC/musttail.c b/clang/test/CodeGen/PowerPC/musttail.c new file mode 100644 index 0000000000000..e3129263d2460 --- /dev/null +++ b/clang/test/CodeGen/PowerPC/musttail.c @@ -0,0 +1,20 @@ +// RUN: %clang_cc1 %s -triple powerpc64-ibm-aix-xcoff -o /dev/null -emit-llvm -verify=aix +// RUN: %clang_cc1 %s -triple powerpc-ibm-aix-xcoff -o /dev/null -emit-llvm -verify=aix +// RUN: %clang_cc1 %s -triple powerpc64-unknown-linux-gnu -o /dev/null -emit-llvm -verify=good +// RUN: %clang_cc1 %s -triple powerpc-unknown-linux-gnu -o /dev/null -emit-llvm -verify=good +// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -o /dev/null -emit-llvm -verify=good +// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -target-feature +pcrelative-memops -o /dev/null -emit-llvm -verify=good +// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -target-feature +longcall -o /dev/null -emit-llvm -verify=longcall +// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -target-feature +pcrelative-memops -target-feature +longcall -o /dev/null -emit-llvm -verify=good + +int foo(int x) { + return x; +} + +int bar(int x) +{ + // good-no-diagnostics + // longcall-error@+2 {{'musttail' attribute for this call is impossible because long calls can not be tail called on PPC}} + // aix-error@+1 {{'musttail' attribute is not supported on AIX}} + [[clang::musttail]] return foo(1); +} diff --git a/clang/test/SemaCXX/attr-musttail-ppc.cpp b/clang/test/SemaCXX/attr-musttail-ppc.cpp deleted file mode 100644 index 3066dc5394efd..0000000000000 --- a/clang/test/SemaCXX/attr-musttail-ppc.cpp +++ /dev/null @@ -1,36 +0,0 @@ -// RUN: %clang_cc1 %s -triple powerpc64-ibm-aix-xcoff -fsyntax-only -verify=aix -// RUN: %clang_cc1 %s -triple powerpc-ibm-aix-xcoff -fsyntax-only -verify=aix -// RUN: %clang_cc1 %s -triple powerpc64-unknown-linux-gnu -fsyntax-only -verify=linux -// RUN: %clang_cc1 %s -triple powerpc-unknown-linux-gnu -fsyntax-only -verify=linux -// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -fsyntax-only -verify=linux -// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -target-feature +pcrelative-memops -fsyntax-only -verify=good -// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -target-feature +longcall -fsyntax-only -verify=longcall -// RUN: %clang_cc1 %s -triple powerpc64le-unknown-linux-gnu -target-feature +pcrelative-memops -target-feature +longcall -fsyntax-only -verify=good - -int good_callee() { - return 0; -} -int good_caller() { - // good-no-diagnostics - // longcall-error@+2 {{'musttail' attribute for this call is impossible because long calls can not be tail called}} - // aix-error@+1 {{'musttail' attribute is not supported on AIX}} - [[clang::musttail]] return good_callee(); -} - -int func(); -int external_call() { - // good-no-diagnostics - // longcall-error@+3 {{'musttail' attribute for this call is impossible because long calls can not be tail called}} - // linux-error@+2 {{'musttail' attribute for this call is impossible because external calls can not be tail called}} - // aix-error@+1 {{'musttail' attribute is not supported on AIX}} - [[clang::musttail]] return func(); -} - -void indirect_call(int r) { - auto Fn = (void (*)(int))1; - // good-no-diagnostics - // longcall-error@+3 {{'musttail' attribute for this call is impossible because long calls can not be tail called}} - // linux-error@+2 {{'musttail' attribute for this call is impossible because indirect calls can not be tail called}} - // aix-error@+1 {{'musttail' attribute is not supported on AIX}} - [[clang::musttail]] return Fn(r); -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits