aeubanks created this revision. Herald added subscribers: ormris, dexonsmith, jdoerfert, pengfei, steven_wu, hiraditya. aeubanks updated this revision to Diff 374663. aeubanks added a comment. aeubanks added reviewers: dblaikie, nickdesaulniers. aeubanks published this revision for review. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits.
update To avoid using the AST when emitting diagnostics, split the "dontcall" attribute into "dontcall-warn" and "dontcall-error", and also add the frontend attribute value as the LLVM attribute value. This gives us all the information to report diagnostics we need from within the IR (aside from access to the original source). One downside is we directly use LLVM's demangler rather than using the existing Clang diagnostic pretty printing of symbols. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D110364 Files: 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 clang/test/Frontend/backend-attribute-error-warning.cpp 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
Index: llvm/test/ThinLTO/X86/dontcall.ll =================================================================== --- llvm/test/ThinLTO/X86/dontcall.ll +++ 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" from the +; attributes match!!! What should happen if we remove "dontcall-error" from the ; definition or declaration of @callee? -; CHECK: call to callee marked "dontcall" +; CHECK: call to callee marked "dontcall-error" ;--- 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" noinline { +define i32 @callee() "dontcall-error" noinline { ret i32 42 } @@ -24,7 +24,7 @@ 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" +declare i32 @callee() "dontcall-error" define i32 @caller() { entry: Index: llvm/test/CodeGen/X86/attr-dontcall.ll =================================================================== --- llvm/test/CodeGen/X86/attr-dontcall.ll +++ llvm/test/CodeGen/X86/attr-dontcall.ll @@ -2,10 +2,24 @@ ; 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" +declare void @foo() "dontcall-error"="e" define void @bar() { call void @foo() ret void } -; CHECK: error: call to foo marked "dontcall" +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"{{$}} Index: llvm/lib/IR/DiagnosticInfo.cpp =================================================================== --- llvm/lib/IR/DiagnosticInfo.cpp +++ llvm/lib/IR/DiagnosticInfo.cpp @@ -400,6 +400,39 @@ void OptimizationRemarkAnalysisFPCommute::anchor() {} void OptimizationRemarkAnalysisAliasing::anchor() {} +void llvm::diagnoseDontCall(const CallInst &CI) { + auto *F = CI.getCalledFunction(); + if (!F) + return; + + if (F->hasFnAttribute("dontcall-error")) { + unsigned LocCookie = 0; + auto A = F->getFnAttribute("dontcall-error"); + if (MDNode *MD = CI.getMetadata("srcloc")) + LocCookie = + mdconst::extract<ConstantInt>(MD->getOperand(0))->getZExtValue(); + DiagnosticInfoDontCall D(F->getName(), A.getValueAsString(), DS_Error, + LocCookie); + F->getContext().diagnose(D); + } + if (F->hasFnAttribute("dontcall-warn")) { + unsigned LocCookie = 0; + auto A = F->getFnAttribute("dontcall-warn"); + if (MDNode *MD = CI.getMetadata("srcloc")) + LocCookie = + mdconst::extract<ConstantInt>(MD->getOperand(0))->getZExtValue(); + DiagnosticInfoDontCall D(F->getName(), A.getValueAsString(), DS_Warning, + LocCookie); + F->getContext().diagnose(D); + } +} + void DiagnosticInfoDontCall::print(DiagnosticPrinter &DP) const { - DP << "call to " << getFunctionName() << " marked \"dontcall\""; + DP << "call to " << getFunctionName() << " marked \"dontcall-"; + if (getSeverity() == DiagnosticSeverity::DS_Error) + DP << "error\""; + else + DP << "warn\""; + if (!getNote().empty()) + DP << ": " << getNote(); } Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp =================================================================== --- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -8036,14 +8036,7 @@ } if (Function *F = I.getCalledFunction()) { - 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); - } + diagnoseDontCall(I); if (F->isDeclaration()) { // Is this an LLVM intrinsic or a target-specific intrinsic? Index: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp =================================================================== --- llvm/lib/CodeGen/SelectionDAG/FastISel.cpp +++ llvm/lib/CodeGen/SelectionDAG/FastISel.cpp @@ -1152,15 +1152,7 @@ CLI.setCallee(RetTy, FuncTy, CI->getCalledOperand(), std::move(Args), *CI) .setTailCall(IsTailCall); - 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); - } + diagnoseDontCall(*CI); return lowerCallTo(CLI); } Index: llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp =================================================================== --- llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp +++ llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp @@ -2336,14 +2336,7 @@ if (CI.isInlineAsm()) return translateInlineAsm(CI, MIRBuilder); - 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); - } + diagnoseDontCall(CI); Intrinsic::ID ID = Intrinsic::not_intrinsic; if (F && F->isIntrinsic()) { Index: llvm/include/llvm/IR/DiagnosticInfo.h =================================================================== --- llvm/include/llvm/IR/DiagnosticInfo.h +++ llvm/include/llvm/IR/DiagnosticInfo.h @@ -33,6 +33,7 @@ // Forward declarations. class DiagnosticPrinter; +class CallInst; class Function; class Instruction; class InstructionCost; @@ -1070,15 +1071,20 @@ } }; +void diagnoseDontCall(const CallInst &CI); + class DiagnosticInfoDontCall : public DiagnosticInfo { StringRef CalleeName; + StringRef Note; unsigned LocCookie; public: - DiagnosticInfoDontCall(StringRef CalleeName, unsigned LocCookie) - : DiagnosticInfo(DK_DontCall, DS_Error), CalleeName(CalleeName), + DiagnosticInfoDontCall(StringRef CalleeName, StringRef Note, + DiagnosticSeverity DS, unsigned LocCookie) + : DiagnosticInfo(DK_DontCall, DS), CalleeName(CalleeName), Note(Note), 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) { Index: llvm/docs/LangRef.rst =================================================================== --- llvm/docs/LangRef.rst +++ llvm/docs/LangRef.rst @@ -1594,12 +1594,18 @@ ``disable_sanitizer_instrumentation`` disables all kinds of instrumentation, taking precedence over the ``sanitize_<name>`` attributes and other compiler flags. -``"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. +``"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. ``"frame-pointer"`` This attribute tells the code generator whether the function should keep the frame pointer. The code generator may emit the frame pointer Index: clang/test/Frontend/backend-attribute-error-warning.cpp =================================================================== --- clang/test/Frontend/backend-attribute-error-warning.cpp +++ clang/test/Frontend/backend-attribute-error-warning.cpp @@ -1,7 +1,5 @@ // 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); @@ -24,14 +22,14 @@ 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 @@ -46,16 +44,16 @@ }; void baz_cpp(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}} - quux(); // enabled-warning {{call to 'quux' declared with 'warning' attribute: oh no quux}} + 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}} + __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 {{'operator int' declared with 'warning' attribute: don't call me!}} + int w = W; // enabled-warning {{Widget::operator int() declared with 'warning' attribute: don't call me!}} } #endif Index: clang/test/Frontend/backend-attribute-error-warning.c =================================================================== --- clang/test/Frontend/backend-attribute-error-warning.c +++ clang/test/Frontend/backend-attribute-error-warning.c @@ -1,7 +1,5 @@ // 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); @@ -24,38 +22,12 @@ 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 Index: clang/test/Frontend/backend-attribute-error-warning-optimize.c =================================================================== --- clang/test/Frontend/backend-attribute-error-warning-optimize.c +++ clang/test/Frontend/backend-attribute-error-warning-optimize.c @@ -8,7 +8,7 @@ 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(); } Index: clang/test/CodeGen/attr-warning.c =================================================================== --- clang/test/CodeGen/attr-warning.c +++ clang/test/CodeGen/attr-warning.c @@ -7,5 +7,5 @@ // CHECK: call void @foo(), !srcloc [[SRCLOC:![0-9]+]] // CHECK: declare{{.*}} void @foo() [[ATTR:#[0-9]+]] -// CHECK: attributes [[ATTR]] = {{{.*}}"dontcall" +// CHECK: attributes [[ATTR]] = {{{.*}}"dontcall-warn"="oh no" // CHECK: [[SRCLOC]] = !{i32 {{[0-9]+}}} Index: clang/test/CodeGen/attr-error.c =================================================================== --- clang/test/CodeGen/attr-error.c +++ clang/test/CodeGen/attr-error.c @@ -7,5 +7,5 @@ // CHECK: call void @foo(), !srcloc [[SRCLOC:![0-9]+]] // CHECK: declare{{.*}} void @foo() [[ATTR:#[0-9]+]] -// CHECK: attributes [[ATTR]] = {{{.*}}"dontcall" +// CHECK: attributes [[ATTR]] = {{{.*}}"dontcall-error"="oh no" // CHECK: [[SRCLOC]] = !{i32 {{[0-9]+}}} Index: clang/lib/CodeGen/CodeGenModule.cpp =================================================================== --- clang/lib/CodeGen/CodeGenModule.cpp +++ clang/lib/CodeGen/CodeGenModule.cpp @@ -2138,8 +2138,12 @@ else if (const auto *SA = FD->getAttr<SectionAttr>()) F->setSection(SA->getName()); - if (FD->hasAttr<ErrorAttr>()) - F->addFnAttr("dontcall"); + 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 we plan on emitting this inline builtin, we can't treat it as a builtin. if (FD->isInlineBuiltinDeclaration()) { Index: clang/lib/CodeGen/CodeGenAction.cpp =================================================================== --- clang/lib/CodeGen/CodeGenAction.cpp +++ clang/lib/CodeGen/CodeGenAction.cpp @@ -27,6 +27,7 @@ #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" @@ -760,30 +761,18 @@ } void BackendConsumer::DontCallDiagHandler(const DiagnosticInfoDontCall &D) { - 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? + 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(); } /// This function is invoked when the backend needs
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits