From 493d6928fe1096aed3af35b0794bf79c00976b19 Mon Sep 17 00:00:00 2001 From: Jon Roelofs Date: Thu, 10 Jun 2021 16:48:45 -0700 Subject: [PATCH] [Remarks] Make memsize remarks report as an analysis, not a missed opportunity. Differential revision: https://reviews.llvm.org/D104078 --- .../llvm/Transforms/Utils/MemoryOpRemark.h | 26 +++++-- llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp | 4 +- llvm/lib/Transforms/Utils/MemoryOpRemark.cpp | 74 +++++++++++-------- llvm/test/CodeGen/AArch64/memsize-remarks.ll | 22 +++--- 4 files changed, 74 insertions(+), 52 deletions(-) diff --git a/llvm/include/llvm/Transforms/Utils/MemoryOpRemark.h b/llvm/include/llvm/Transforms/Utils/MemoryOpRemark.h index 54ec87c82073..7b4a1cdbf4fd 100644 --- a/llvm/include/llvm/Transforms/Utils/MemoryOpRemark.h +++ b/llvm/include/llvm/Transforms/Utils/MemoryOpRemark.h @@ -17,6 +17,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Analysis/TargetLibraryInfo.h" +#include "llvm/IR/DiagnosticInfo.h" namespace llvm { @@ -28,6 +29,7 @@ class IntrinsicInst; class Value; class OptimizationRemarkEmitter; class OptimizationRemarkMissed; +class OptimizationRemarkAnalysis; class StoreInst; // FIXME: Once we get to more remarks like this one, we need to re-evaluate how @@ -50,12 +52,17 @@ struct MemoryOpRemark { void visit(const Instruction *I); protected: - virtual std::string explainSource(StringRef Type); + virtual std::string explainSource(StringRef Type) const; enum RemarkKind { RK_Store, RK_Unknown, RK_IntrinsicCall, RK_Call }; - virtual StringRef remarkName(RemarkKind RK); + virtual StringRef remarkName(RemarkKind RK) const; + + virtual DiagnosticKind diagnosticKind() const { return DK_OptimizationRemarkAnalysis; } private: + template + std::unique_ptr makeRemark(Ts... Args); + /// Emit a remark using information from the store's destination, size, etc. void visitStore(const StoreInst &SI); /// Emit a generic auto-init remark. @@ -68,13 +75,13 @@ private: /// Add callee information to a remark: whether it's known, the function name, /// etc. template - void visitCallee(FTy F, bool KnownLibCall, OptimizationRemarkMissed &R); + void visitCallee(FTy F, bool KnownLibCall, DiagnosticInfoIROptimization &R); /// Add operand information to a remark based on knowledge we have for known /// libcalls. void visitKnownLibCall(const CallInst &CI, LibFunc LF, - OptimizationRemarkMissed &R); + DiagnosticInfoIROptimization &R); /// Add the memory operation size to a remark. - void visitSizeOperand(Value *V, OptimizationRemarkMissed &R); + void visitSizeOperand(Value *V, DiagnosticInfoIROptimization &R); struct VariableInfo { Optional Name; @@ -84,7 +91,7 @@ private: /// Gather more information about \p V as a variable. This can be debug info, /// information from the alloca, etc. Since \p V can represent more than a /// single variable, they will all be added to the remark. - void visitPtr(Value *V, bool IsSrc, OptimizationRemarkMissed &R); + void visitPtr(Value *V, bool IsSrc, DiagnosticInfoIROptimization &R); void visitVariable(const Value *V, SmallVectorImpl &Result); }; @@ -98,8 +105,11 @@ struct AutoInitRemark : public MemoryOpRemark { static bool canHandle(const Instruction *I); protected: - virtual std::string explainSource(StringRef Type) override; - virtual StringRef remarkName(RemarkKind RK) override; + virtual std::string explainSource(StringRef Type) const override; + virtual StringRef remarkName(RemarkKind RK) const override; + virtual DiagnosticKind diagnosticKind() const override { + return DK_OptimizationRemarkMissed; + } }; } // namespace llvm diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp index 6d205677e15f..b5d60ac930fe 100644 --- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp +++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp @@ -1824,7 +1824,7 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID, const Function &F = *MI->getParent()->getParent(); auto &TLI = getAnalysis().getTLI(F); if (MemoryOpRemark::canHandle(MI, TLI)) { - MemoryOpRemark R(*ORE, "memsize", *DL, TLI); + MemoryOpRemark R(*ORE, "gisel-irtranslator-memsize", *DL, TLI); R.visit(MI); } } @@ -2263,7 +2263,7 @@ bool IRTranslator::translateCallBase(const CallBase &CB, const Function &F = *CI->getParent()->getParent(); auto &TLI = getAnalysis().getTLI(F); if (MemoryOpRemark::canHandle(CI, TLI)) { - MemoryOpRemark R(*ORE, "memsize", *DL, TLI); + MemoryOpRemark R(*ORE, "gisel-irtranslator-memsize", *DL, TLI); R.visit(CI); } } diff --git a/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp b/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp index 77f8a98004e4..8836fe220ad8 100644 --- a/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp +++ b/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp @@ -105,11 +105,11 @@ void MemoryOpRemark::visit(const Instruction *I) { visitUnknown(*I); } -std::string MemoryOpRemark::explainSource(StringRef Type) { +std::string MemoryOpRemark::explainSource(StringRef Type) const { return (Type + ".").str(); } -StringRef MemoryOpRemark::remarkName(RemarkKind RK) { +StringRef MemoryOpRemark::remarkName(RemarkKind RK) const { switch (RK) { case RK_Store: return "MemoryOpStore"; @@ -125,7 +125,7 @@ StringRef MemoryOpRemark::remarkName(RemarkKind RK) { static void inlineVolatileOrAtomicWithExtraArgs(bool *Inline, bool Volatile, bool Atomic, - OptimizationRemarkMissed &R) { + DiagnosticInfoIROptimization &R) { if (Inline && *Inline) R << " Inlined: " << NV("StoreInlined", true) << "."; if (Volatile) @@ -150,23 +150,36 @@ static Optional getSizeInBytes(Optional SizeInBits) { return *SizeInBits / 8; } +template +std::unique_ptr +MemoryOpRemark::makeRemark(Ts... Args) { + switch (diagnosticKind()) { + case DK_OptimizationRemarkAnalysis: + return std::make_unique(Args...); + case DK_OptimizationRemarkMissed: + return std::make_unique(Args...); + default: + llvm_unreachable("unexpected DiagnosticKind"); + } +} + void MemoryOpRemark::visitStore(const StoreInst &SI) { bool Volatile = SI.isVolatile(); bool Atomic = SI.isAtomic(); int64_t Size = DL.getTypeStoreSize(SI.getOperand(0)->getType()); - OptimizationRemarkMissed R(RemarkPass.data(), remarkName(RK_Store), &SI); - R << explainSource("Store") << "\nStore size: " << NV("StoreSize", Size) - << " bytes."; - visitPtr(SI.getOperand(1), /*IsRead=*/false, R); - inlineVolatileOrAtomicWithExtraArgs(nullptr, Volatile, Atomic, R); - ORE.emit(R); + auto R = makeRemark(RemarkPass.data(), remarkName(RK_Store), &SI); + *R << explainSource("Store") << "\nStore size: " << NV("StoreSize", Size) + << " bytes."; + visitPtr(SI.getOperand(1), /*IsRead=*/false, *R); + inlineVolatileOrAtomicWithExtraArgs(nullptr, Volatile, Atomic, *R); + ORE.emit(*R); } void MemoryOpRemark::visitUnknown(const Instruction &I) { - OptimizationRemarkMissed R(RemarkPass.data(), remarkName(RK_Unknown), &I); - R << explainSource("Initialization"); - ORE.emit(R); + auto R = makeRemark(RemarkPass.data(), remarkName(RK_Unknown), &I); + *R << explainSource("Initialization"); + ORE.emit(*R); } void MemoryOpRemark::visitIntrinsicCall(const IntrinsicInst &II) { @@ -203,10 +216,9 @@ void MemoryOpRemark::visitIntrinsicCall(const IntrinsicInst &II) { return visitUnknown(II); } - OptimizationRemarkMissed R(RemarkPass.data(), remarkName(RK_IntrinsicCall), - &II); - visitCallee(StringRef(CallTo), /*KnownLibCall=*/true, R); - visitSizeOperand(II.getOperand(2), R); + auto R = makeRemark(RemarkPass.data(), remarkName(RK_IntrinsicCall), &II); + visitCallee(StringRef(CallTo), /*KnownLibCall=*/true, *R); + visitSizeOperand(II.getOperand(2), *R); auto *CIVolatile = dyn_cast(II.getOperand(3)); // No such thing as a memory intrinsic that is both atomic and volatile. @@ -216,16 +228,16 @@ void MemoryOpRemark::visitIntrinsicCall(const IntrinsicInst &II) { case Intrinsic::memcpy: case Intrinsic::memmove: case Intrinsic::memcpy_element_unordered_atomic: - visitPtr(II.getOperand(1), /*IsRead=*/true, R); - visitPtr(II.getOperand(0), /*IsRead=*/false, R); + visitPtr(II.getOperand(1), /*IsRead=*/true, *R); + visitPtr(II.getOperand(0), /*IsRead=*/false, *R); break; case Intrinsic::memset: case Intrinsic::memset_element_unordered_atomic: - visitPtr(II.getOperand(0), /*IsRead=*/false, R); + visitPtr(II.getOperand(0), /*IsRead=*/false, *R); break; } - inlineVolatileOrAtomicWithExtraArgs(&Inline, Volatile, Atomic, R); - ORE.emit(R); + inlineVolatileOrAtomicWithExtraArgs(&Inline, Volatile, Atomic, *R); + ORE.emit(*R); } void MemoryOpRemark::visitCall(const CallInst &CI) { @@ -235,15 +247,15 @@ void MemoryOpRemark::visitCall(const CallInst &CI) { LibFunc LF; bool KnownLibCall = TLI.getLibFunc(*F, LF) && TLI.has(LF); - OptimizationRemarkMissed R(RemarkPass.data(), remarkName(RK_Call), &CI); - visitCallee(F, KnownLibCall, R); - visitKnownLibCall(CI, LF, R); - ORE.emit(R); + auto R = makeRemark(RemarkPass.data(), remarkName(RK_Call), &CI); + visitCallee(F, KnownLibCall, *R); + visitKnownLibCall(CI, LF, *R); + ORE.emit(*R); } template void MemoryOpRemark::visitCallee(FTy F, bool KnownLibCall, - OptimizationRemarkMissed &R) { + DiagnosticInfoIROptimization &R) { R << "Call to "; if (!KnownLibCall) R << NV("UnknownLibCall", "unknown") << " function "; @@ -251,7 +263,7 @@ void MemoryOpRemark::visitCallee(FTy F, bool KnownLibCall, } void MemoryOpRemark::visitKnownLibCall(const CallInst &CI, LibFunc LF, - OptimizationRemarkMissed &R) { + DiagnosticInfoIROptimization &R) { switch (LF) { default: return; @@ -278,7 +290,7 @@ void MemoryOpRemark::visitKnownLibCall(const CallInst &CI, LibFunc LF, } } -void MemoryOpRemark::visitSizeOperand(Value *V, OptimizationRemarkMissed &R) { +void MemoryOpRemark::visitSizeOperand(Value *V, DiagnosticInfoIROptimization &R) { if (auto *Len = dyn_cast(V)) { uint64_t Size = Len->getZExtValue(); R << " Memory operation size: " << NV("StoreSize", Size) << " bytes."; @@ -335,7 +347,7 @@ void MemoryOpRemark::visitVariable(const Value *V, Result.push_back(std::move(Var)); } -void MemoryOpRemark::visitPtr(Value *Ptr, bool IsRead, OptimizationRemarkMissed &R) { +void MemoryOpRemark::visitPtr(Value *Ptr, bool IsRead, DiagnosticInfoIROptimization &R) { // Find if Ptr is a known variable we can give more information on. SmallVector Objects; getUnderlyingObjectsForCodeGen(Ptr, Objects); @@ -377,11 +389,11 @@ bool AutoInitRemark::canHandle(const Instruction *I) { }); } -std::string AutoInitRemark::explainSource(StringRef Type) { +std::string AutoInitRemark::explainSource(StringRef Type) const { return (Type + " inserted by -ftrivial-auto-var-init.").str(); } -StringRef AutoInitRemark::remarkName(RemarkKind RK) { +StringRef AutoInitRemark::remarkName(RemarkKind RK) const { switch (RK) { case RK_Store: return "AutoInitStore"; diff --git a/llvm/test/CodeGen/AArch64/memsize-remarks.ll b/llvm/test/CodeGen/AArch64/memsize-remarks.ll index d505c75c0776..2d9ef520d570 100644 --- a/llvm/test/CodeGen/AArch64/memsize-remarks.ll +++ b/llvm/test/CodeGen/AArch64/memsize-remarks.ll @@ -1,4 +1,4 @@ -; RUN: llc %s -pass-remarks-missed=memsize -pass-remarks-output=%t.opt.yaml -pass-remarks-filter=memsize -global-isel -o /dev/null 2>&1 | FileCheck %s --check-prefix=GISEL --implicit-check-not=GISEL +; RUN: llc %s -pass-remarks-analysis=gisel-irtranslator-memsize -pass-remarks-output=%t.opt.yaml -pass-remarks-filter=gisel-irtranslator-memsize -global-isel -o /dev/null 2>&1 | FileCheck %s --check-prefix=GISEL --implicit-check-not=GISEL ; RUN: cat %t.opt.yaml | FileCheck -check-prefix=YAML %s source_filename = "memsize.c" @@ -143,8 +143,8 @@ define void @known_call_with_dereferenceable_bytes(i8* dereferenceable(42) %dst, ; GISEL: Call to memset. Memory operation size: 1 bytes. ; GISEL-NOT: Read Variables: ; GISEL-NEXT: Written Variables: (42 bytes). -; YAML: --- !Missed -; YAML: Pass: memsize +; YAML: --- !Analysis +; YAML: gisel-irtranslator-memsize ; YAML: Name: MemoryOpIntrinsicCall ; YAML-LABEL: Function: known_call_with_dereferenceable_bytes ; YAML-NEXT: Args: @@ -175,8 +175,8 @@ define void @known_call_with_dereferenceable_bytes(i8* dereferenceable(42) %dst, ; GISEL: Call to memcpy. Memory operation size: 1 bytes. ; GISEL-NEXT: Read Variables: (314 bytes). ; GISEL-NEXT: Written Variables: (42 bytes). -; YAML: --- !Missed -; YAML: Pass: memsize +; YAML: --- !Analysis +; YAML: gisel-irtranslator-memsize ; YAML: Name: MemoryOpIntrinsicCall ; YAML-LABEL: Function: known_call_with_dereferenceable_bytes ; YAML-NEXT: Args: @@ -213,8 +213,8 @@ define void @known_call_with_dereferenceable_bytes(i8* dereferenceable(42) %dst, ; GISEL: Call to memmove. Memory operation size: 1 bytes. ; GISEL-NEXT: Read Variables: (314 bytes). ; GISEL-NEXT: Written Variables: (42 bytes). -; YAML: --- !Missed -; YAML: Pass: memsize +; YAML: --- !Analysis +; YAML: gisel-irtranslator-memsize ; YAML: Name: MemoryOpIntrinsicCall ; YAML-LABEL: Function: known_call_with_dereferenceable_bytes ; YAML-NEXT: Args: @@ -251,8 +251,8 @@ define void @known_call_with_dereferenceable_bytes(i8* dereferenceable(42) %dst, ; GISEL: Call to bzero. Memory operation size: 1 bytes. ; GISEL-NOT: Read Variables: ; GISEL-NEXT: Written Variables: (42 bytes). -; YAML: --- !Missed -; YAML: Pass: memsize +; YAML: --- !Analysis +; YAML: gisel-irtranslator-memsize ; YAML: Name: MemoryOpCall ; YAML-LABEL: Function: known_call_with_dereferenceable_bytes ; YAML-NEXT: Args: @@ -274,8 +274,8 @@ define void @known_call_with_dereferenceable_bytes(i8* dereferenceable(42) %dst, ; GISEL: Call to bcopy. Memory operation size: 1 bytes. ; GISEL-NEXT: Read Variables: (314 bytes). ; GISEL-NEXT: Written Variables: (42 bytes). -; YAML: --- !Missed -; YAML: Pass: memsize +; YAML: --- !Analysis +; YAML: gisel-irtranslator-memsize ; YAML: Name: MemoryOpCall ; YAML-LABEL: Function: known_call_with_dereferenceable_bytes ; YAML-NEXT: Args: