Author: Arthur Eubanks Date: 2021-09-28T14:49:27-07:00 New Revision: 7833d20f1fd575fac89ce76822ffd561a40552e5
URL: https://github.com/llvm/llvm-project/commit/7833d20f1fd575fac89ce76822ffd561a40552e5 DIFF: https://github.com/llvm/llvm-project/commit/7833d20f1fd575fac89ce76822ffd561a40552e5.diff LOG: Revert "[clang] Rework dontcall attributes" This reverts commit 2943071e2ee0c7f31f34062a44d12aeb0e3a66fd. Breaks bots Added: Modified: clang/lib/CodeGen/CodeGenAction.cpp clang/lib/CodeGen/CodeGenModule.cpp clang/test/CodeGen/attr-error.c clang/test/CodeGen/attr-warning.c clang/test/Frontend/backend-attribute-error-warning-optimize.c clang/test/Frontend/backend-attribute-error-warning.c llvm/docs/LangRef.rst llvm/include/llvm/IR/DiagnosticInfo.h llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp llvm/lib/CodeGen/SelectionDAG/FastISel.cpp llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp llvm/lib/IR/DiagnosticInfo.cpp llvm/test/CodeGen/X86/attr-dontcall.ll llvm/test/ThinLTO/X86/dontcall.ll Removed: clang/test/Frontend/backend-attribute-error-warning.cpp ################################################################################ diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp index 6e1d7f320673c..789c06a15b71c 100644 --- a/clang/lib/CodeGen/CodeGenAction.cpp +++ b/clang/lib/CodeGen/CodeGenAction.cpp @@ -27,7 +27,6 @@ #include "clang/Lex/Preprocessor.h" #include "llvm/Bitcode/BitcodeReader.h" #include "llvm/CodeGen/MachineOptimizationRemarkEmitter.h" -#include "llvm/Demangle/Demangle.h" #include "llvm/IR/DebugInfo.h" #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/DiagnosticPrinter.h" @@ -761,18 +760,30 @@ void BackendConsumer::OptimizationFailureHandler( } void BackendConsumer::DontCallDiagHandler(const DiagnosticInfoDontCall &D) { - SourceLocation LocCookie = - SourceLocation::getFromRawEncoding(D.getLocCookie()); - - // FIXME: we can't yet diagnose indirect calls. When/if we can, we - // should instead assert that LocCookie.isValid(). - if (!LocCookie.isValid()) - return; - - Diags.Report(LocCookie, D.getSeverity() == DiagnosticSeverity::DS_Error - ? diag::err_fe_backend_error_attr - : diag::warn_fe_backend_warning_attr) - << llvm::demangle(D.getFunctionName().str()) << D.getNote(); + if (const Decl *DE = Gen->GetDeclForMangledName(D.getFunctionName())) + if (const auto *FD = dyn_cast<FunctionDecl>(DE)) { + assert(FD->hasAttr<ErrorAttr>() && + "expected error or warning function attribute"); + + if (const auto *EA = FD->getAttr<ErrorAttr>()) { + assert((EA->isError() || EA->isWarning()) && + "ErrorAttr neither error or warning"); + + SourceLocation LocCookie = + SourceLocation::getFromRawEncoding(D.getLocCookie()); + + // FIXME: we can't yet diagnose indirect calls. When/if we can, we + // should instead assert that LocCookie.isValid(). + if (!LocCookie.isValid()) + return; + + Diags.Report(LocCookie, EA->isError() + ? diag::err_fe_backend_error_attr + : diag::warn_fe_backend_warning_attr) + << FD << EA->getUserDiagnostic(); + } + } + // TODO: assert if DE or FD were nullptr? } /// This function is invoked when the backend needs diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index d26b1343247f0..83418014f0b5c 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -2138,12 +2138,8 @@ void CodeGenModule::SetFunctionAttributes(GlobalDecl GD, llvm::Function *F, else if (const auto *SA = FD->getAttr<SectionAttr>()) F->setSection(SA->getName()); - if (const auto *EA = FD->getAttr<ErrorAttr>()) { - if (EA->isError()) - F->addFnAttr("dontcall-error", EA->getUserDiagnostic()); - else if (EA->isWarning()) - F->addFnAttr("dontcall-warn", EA->getUserDiagnostic()); - } + if (FD->hasAttr<ErrorAttr>()) + F->addFnAttr("dontcall"); // If we plan on emitting this inline builtin, we can't treat it as a builtin. if (FD->isInlineBuiltinDeclaration()) { diff --git a/clang/test/CodeGen/attr-error.c b/clang/test/CodeGen/attr-error.c index a1b63ab9fa9e5..da56793a23920 100644 --- a/clang/test/CodeGen/attr-error.c +++ b/clang/test/CodeGen/attr-error.c @@ -7,5 +7,5 @@ void bar(void) { // CHECK: call void @foo(), !srcloc [[SRCLOC:![0-9]+]] // CHECK: declare{{.*}} void @foo() [[ATTR:#[0-9]+]] -// CHECK: attributes [[ATTR]] = {{{.*}}"dontcall-error"="oh no" +// CHECK: attributes [[ATTR]] = {{{.*}}"dontcall" // CHECK: [[SRCLOC]] = !{i32 {{[0-9]+}}} diff --git a/clang/test/CodeGen/attr-warning.c b/clang/test/CodeGen/attr-warning.c index 5c89066aff75a..daa53b6616513 100644 --- a/clang/test/CodeGen/attr-warning.c +++ b/clang/test/CodeGen/attr-warning.c @@ -7,5 +7,5 @@ void bar(void) { // CHECK: call void @foo(), !srcloc [[SRCLOC:![0-9]+]] // CHECK: declare{{.*}} void @foo() [[ATTR:#[0-9]+]] -// CHECK: attributes [[ATTR]] = {{{.*}}"dontcall-warn"="oh no" +// CHECK: attributes [[ATTR]] = {{{.*}}"dontcall" // CHECK: [[SRCLOC]] = !{i32 {{[0-9]+}}} diff --git a/clang/test/Frontend/backend-attribute-error-warning-optimize.c b/clang/test/Frontend/backend-attribute-error-warning-optimize.c index 668a0a4ca71f3..d3951e3b6b1f5 100644 --- a/clang/test/Frontend/backend-attribute-error-warning-optimize.c +++ b/clang/test/Frontend/backend-attribute-error-warning-optimize.c @@ -8,7 +8,7 @@ int x(void) { return 8 % 2 == 1; } void baz(void) { - foo(); // expected-error {{call to foo declared with 'error' attribute: oh no foo}} + foo(); // expected-error {{call to 'foo' declared with 'error' attribute: oh no foo}} if (x()) bar(); } diff --git a/clang/test/Frontend/backend-attribute-error-warning.c b/clang/test/Frontend/backend-attribute-error-warning.c index a1450b46ba87d..4e96f771d54cb 100644 --- a/clang/test/Frontend/backend-attribute-error-warning.c +++ b/clang/test/Frontend/backend-attribute-error-warning.c @@ -1,5 +1,7 @@ // RUN: %clang_cc1 -verify=expected,enabled -emit-codegen-only %s +// RUN: %clang_cc1 -verify=expected,enabled -emit-codegen-only %s -x c++ // RUN: %clang_cc1 -verify -emit-codegen-only -Wno-attribute-warning %s +// RUN: %clang_cc1 -verify -emit-codegen-only -Wno-attribute-warning %s -x c++ __attribute__((error("oh no foo"))) void foo(void); @@ -22,12 +24,38 @@ void // expected-note@-1 {{previous a duplicate_warnings(void); void baz(void) { - foo(); // expected-error {{call to foo declared with 'error' attribute: oh no foo}} + foo(); // expected-error {{call to 'foo' declared with 'error' attribute: oh no foo}} if (x()) - bar(); // expected-error {{call to bar declared with 'error' attribute: oh no bar}} + bar(); // expected-error {{call to 'bar' declared with 'error' attribute: oh no bar}} - quux(); // enabled-warning {{call to quux declared with 'warning' attribute: oh no quux}} - __compiletime_assert_455(); // expected-error {{call to __compiletime_assert_455 declared with 'error' attribute: demangle me}} - duplicate_errors(); // expected-error {{call to duplicate_errors declared with 'error' attribute: two}} - duplicate_warnings(); // enabled-warning {{call to duplicate_warnings declared with 'warning' attribute: two}} + quux(); // enabled-warning {{call to 'quux' declared with 'warning' attribute: oh no quux}} + __compiletime_assert_455(); // expected-error {{call to '__compiletime_assert_455' declared with 'error' attribute: demangle me}} + duplicate_errors(); // expected-error {{call to 'duplicate_errors' declared with 'error' attribute: two}} + duplicate_warnings(); // enabled-warning {{call to 'duplicate_warnings' declared with 'warning' attribute: two}} } + +#ifdef __cplusplus +template <typename T> +__attribute__((error("demangle me, too"))) +T +nocall(T t); + +struct Widget { + __attribute__((warning("don't call me!"))) + operator int() { return 42; } +}; + +void baz_cpp(void) { + foo(); // expected-error {{call to 'foo' declared with 'error' attribute: oh no foo}} + if (x()) + bar(); // expected-error {{call to 'bar' declared with 'error' attribute: oh no bar}} + quux(); // enabled-warning {{call to 'quux' declared with 'warning' attribute: oh no quux}} + + // Test that we demangle correctly in the diagnostic for C++. + __compiletime_assert_455(); // expected-error {{call to '__compiletime_assert_455' declared with 'error' attribute: demangle me}} + nocall<int>(42); // expected-error {{call to 'nocall<int>' declared with 'error' attribute: demangle me, too}} + + Widget W; + int w = W; // enabled-warning {{'operator int' declared with 'warning' attribute: don't call me!}} +} +#endif diff --git a/clang/test/Frontend/backend-attribute-error-warning.cpp b/clang/test/Frontend/backend-attribute-error-warning.cpp deleted file mode 100644 index 0338fe4131d17..0000000000000 --- a/clang/test/Frontend/backend-attribute-error-warning.cpp +++ /dev/null @@ -1,59 +0,0 @@ -// RUN: %clang_cc1 -verify=expected,enabled -emit-codegen-only %s -// RUN: %clang_cc1 -verify -emit-codegen-only -Wno-attribute-warning %s - -__attribute__((error("oh no foo"))) void foo(void); - -__attribute__((error("oh no bar"))) void bar(void); - -int x(void) { - return 8 % 2 == 1; -} - -__attribute__((warning("oh no quux"))) void quux(void); - -__attribute__((error("demangle me"))) void __compiletime_assert_455(void); - -__attribute__((error("one"), error("two"))) // expected-warning {{attribute 'error' is already applied with diff erent arguments}} -void // expected-note@-1 {{previous attribute is here}} -duplicate_errors(void); - -__attribute__((warning("one"), warning("two"))) // expected-warning {{attribute 'warning' is already applied with diff erent arguments}} -void // expected-note@-1 {{previous attribute is here}} -duplicate_warnings(void); - -void baz(void) { - foo(); // expected-error {{call to foo() declared with 'error' attribute: oh no foo}} - if (x()) - bar(); // expected-error {{call to bar() declared with 'error' attribute: oh no bar}} - - quux(); // enabled-warning {{call to quux() declared with 'warning' attribute: oh no quux}} - __compiletime_assert_455(); // expected-error {{call to __compiletime_assert_455() declared with 'error' attribute: demangle me}} - duplicate_errors(); // expected-error {{call to duplicate_errors() declared with 'error' attribute: two}} - duplicate_warnings(); // enabled-warning {{call to duplicate_warnings() declared with 'warning' attribute: two}} -} - -#ifdef __cplusplus -template <typename T> -__attribute__((error("demangle me, too"))) -T -nocall(T t); - -struct Widget { - __attribute__((warning("don't call me!"))) - operator int() { return 42; } -}; - -void baz_cpp(void) { - foo(); // expected-error {{call to foo() declared with 'error' attribute: oh no foo}} - if (x()) - bar(); // expected-error {{call to bar() declared with 'error' attribute: oh no bar}} - quux(); // enabled-warning {{call to quux() declared with 'warning' attribute: oh no quux}} - - // Test that we demangle correctly in the diagnostic for C++. - __compiletime_assert_455(); // expected-error {{call to __compiletime_assert_455() declared with 'error' attribute: demangle me}} - nocall<int>(42); // expected-error {{call to int nocall<int>(int) declared with 'error' attribute: demangle me, too}} - - Widget W; - int w = W; // enabled-warning {{Widget::operator int() declared with 'warning' attribute: don't call me!}} -} -#endif diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index a40a1618efe28..8240b8d3597a1 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -1594,18 +1594,12 @@ example: ``disable_sanitizer_instrumentation`` disables all kinds of instrumentation, taking precedence over the ``sanitize_<name>`` attributes and other compiler flags. -``"dontcall-error"`` - This attribute denotes that an error diagnostic should be emitted when a - call of a function with this attribute is not eliminated via optimization. - Front ends can provide optional ``srcloc`` metadata nodes on call sites of - such callees to attach information about where in the source language such a - call came from. A string value can be provided as a note. -``"dontcall-warn"`` - This attribute denotes that a warning diagnostic should be emitted when a - call of a function with this attribute is not eliminated via optimization. - Front ends can provide optional ``srcloc`` metadata nodes on call sites of - such callees to attach information about where in the source language such a - call came from. A string value can be provided as a note. +``"dontcall"`` + This attribute denotes that a diagnostic should be emitted when a call of a + function with this attribute is not eliminated via optimization. Front ends + can provide optional ``srcloc`` metadata nodes on call sites of such + callees to attach information about where in the source language such a + call came from. ``"frame-pointer"`` This attribute tells the code generator whether the function should keep the frame pointer. The code generator may emit the frame pointer diff --git a/llvm/include/llvm/IR/DiagnosticInfo.h b/llvm/include/llvm/IR/DiagnosticInfo.h index 73b0be43e1367..44047d9f1211c 100644 --- a/llvm/include/llvm/IR/DiagnosticInfo.h +++ b/llvm/include/llvm/IR/DiagnosticInfo.h @@ -33,7 +33,6 @@ namespace llvm { // Forward declarations. class DiagnosticPrinter; -class CallInst; class Function; class Instruction; class InstructionCost; @@ -1071,20 +1070,15 @@ class DiagnosticInfoSrcMgr : public DiagnosticInfo { } }; -void diagnoseDontCall(const CallInst &CI); - class DiagnosticInfoDontCall : public DiagnosticInfo { StringRef CalleeName; - StringRef Note; unsigned LocCookie; public: - DiagnosticInfoDontCall(StringRef CalleeName, StringRef Note, - DiagnosticSeverity DS, unsigned LocCookie) - : DiagnosticInfo(DK_DontCall, DS), CalleeName(CalleeName), Note(Note), + DiagnosticInfoDontCall(StringRef CalleeName, unsigned LocCookie) + : DiagnosticInfo(DK_DontCall, DS_Error), CalleeName(CalleeName), LocCookie(LocCookie) {} StringRef getFunctionName() const { return CalleeName; } - StringRef getNote() const { return Note; } unsigned getLocCookie() const { return LocCookie; } void print(DiagnosticPrinter &DP) const override; static bool classof(const DiagnosticInfo *DI) { diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp index 13a0da1bff1c8..e27abdc9559c8 100644 --- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp +++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp @@ -2344,7 +2344,14 @@ bool IRTranslator::translateCall(const User &U, MachineIRBuilder &MIRBuilder) { if (CI.isInlineAsm()) return translateInlineAsm(CI, MIRBuilder); - diagnoseDontCall(CI); + if (F && F->hasFnAttribute("dontcall")) { + unsigned LocCookie = 0; + if (MDNode *MD = CI.getMetadata("srcloc")) + LocCookie = + mdconst::extract<ConstantInt>(MD->getOperand(0))->getZExtValue(); + DiagnosticInfoDontCall D(F->getName(), LocCookie); + F->getContext().diagnose(D); + } Intrinsic::ID ID = Intrinsic::not_intrinsic; if (F && F->isIntrinsic()) { diff --git a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp index 7af0db12b7c80..251e6989b99ec 100644 --- a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp @@ -1152,7 +1152,15 @@ bool FastISel::lowerCall(const CallInst *CI) { CLI.setCallee(RetTy, FuncTy, CI->getCalledOperand(), std::move(Args), *CI) .setTailCall(IsTailCall); - diagnoseDontCall(*CI); + if (const Function *F = CI->getCalledFunction()) + if (F->hasFnAttribute("dontcall")) { + unsigned LocCookie = 0; + if (MDNode *MD = CI->getMetadata("srcloc")) + LocCookie = + mdconst::extract<ConstantInt>(MD->getOperand(0))->getZExtValue(); + DiagnosticInfoDontCall D(F->getName(), LocCookie); + F->getContext().diagnose(D); + } return lowerCallTo(CLI); } diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 7c16652451b41..6b09d39f8b904 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -8036,7 +8036,14 @@ void SelectionDAGBuilder::visitCall(const CallInst &I) { } if (Function *F = I.getCalledFunction()) { - diagnoseDontCall(I); + if (F->hasFnAttribute("dontcall")) { + unsigned LocCookie = 0; + if (MDNode *MD = I.getMetadata("srcloc")) + LocCookie = + mdconst::extract<ConstantInt>(MD->getOperand(0))->getZExtValue(); + DiagnosticInfoDontCall D(F->getName(), LocCookie); + DAG.getContext()->diagnose(D); + } if (F->isDeclaration()) { // Is this an LLVM intrinsic or a target-specific intrinsic? diff --git a/llvm/lib/IR/DiagnosticInfo.cpp b/llvm/lib/IR/DiagnosticInfo.cpp index 3d0f13bcf5771..4492e2efe81e5 100644 --- a/llvm/lib/IR/DiagnosticInfo.cpp +++ b/llvm/lib/IR/DiagnosticInfo.cpp @@ -400,34 +400,6 @@ std::string DiagnosticInfoOptimizationBase::getMsg() const { void OptimizationRemarkAnalysisFPCommute::anchor() {} void OptimizationRemarkAnalysisAliasing::anchor() {} -void llvm::diagnoseDontCall(const CallInst &CI) { - auto *F = CI.getCalledFunction(); - if (!F) - return; - - for (int i = 0; i != 2; ++i) { - auto AttrName = i == 0 ? "dontcall-error" : "dontcall-warn"; - auto Sev = i == 0 ? DS_Error : DS_Warning; - - if (F->hasFnAttribute(AttrName)) { - unsigned LocCookie = 0; - auto A = F->getFnAttribute(AttrName); - if (MDNode *MD = CI.getMetadata("srcloc")) - LocCookie = - mdconst::extract<ConstantInt>(MD->getOperand(0))->getZExtValue(); - DiagnosticInfoDontCall D(F->getName(), A.getValueAsString(), Sev, - LocCookie); - F->getContext().diagnose(D); - } - } -} - void DiagnosticInfoDontCall::print(DiagnosticPrinter &DP) const { - DP << "call to " << getFunctionName() << " marked \"dontcall-"; - if (getSeverity() == DiagnosticSeverity::DS_Error) - DP << "error\""; - else - DP << "warn\""; - if (!getNote().empty()) - DP << ": " << getNote(); + DP << "call to " << getFunctionName() << " marked \"dontcall\""; } diff --git a/llvm/test/CodeGen/X86/attr-dontcall.ll b/llvm/test/CodeGen/X86/attr-dontcall.ll index 4c2595f7d4ee8..7cdac32e7c816 100644 --- a/llvm/test/CodeGen/X86/attr-dontcall.ll +++ b/llvm/test/CodeGen/X86/attr-dontcall.ll @@ -2,24 +2,10 @@ ; RUN: not llc -mtriple=x86_64 -global-isel=0 -fast-isel=1 -stop-after=finalize-isel < %s 2>&1 | FileCheck %s ; RUN: not llc -mtriple=x86_64 -global-isel=1 -fast-isel=0 -stop-after=irtranslator -global-isel-abort=0 < %s 2>&1 | FileCheck %s -declare void @foo() "dontcall-error"="e" +declare void @foo() "dontcall" define void @bar() { call void @foo() ret void } -declare void @foo2() "dontcall-warn"="w" -define void @bar2() { - call void @foo2() - ret void -} - -declare void @foo3() "dontcall-warn" -define void @bar3() { - call void @foo3() - ret void -} - -; CHECK: error: call to foo marked "dontcall-error": e -; CHECK: warning: call to foo2 marked "dontcall-warn": w -; CHECK: warning: call to foo3 marked "dontcall-warn"{{$}} +; CHECK: error: call to foo marked "dontcall" diff --git a/llvm/test/ThinLTO/X86/dontcall.ll b/llvm/test/ThinLTO/X86/dontcall.ll index baacf29d8aff3..ef5eae9b4b829 100644 --- a/llvm/test/ThinLTO/X86/dontcall.ll +++ b/llvm/test/ThinLTO/X86/dontcall.ll @@ -7,16 +7,16 @@ ; RUN: -r=%t/b.bc,caller,px ; TODO: As part of LTO, we check that types match, but *we don't yet check that -; attributes match!!! What should happen if we remove "dontcall-error" from the +; attributes match!!! What should happen if we remove "dontcall" from the ; definition or declaration of @callee? -; CHECK: call to callee marked "dontcall-error" +; CHECK: call to callee marked "dontcall" ;--- a.s target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-unknown-linux-gnu" -define i32 @callee() "dontcall-error" noinline { +define i32 @callee() "dontcall" noinline { ret i32 42 } @@ -24,7 +24,7 @@ define i32 @callee() "dontcall-error" noinline { target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-unknown-linux-gnu" -declare i32 @callee() "dontcall-error" +declare i32 @callee() "dontcall" define i32 @caller() { entry: _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
