From 093624c347baf30d43a40f8916f2c89145411e95 Mon Sep 17 00:00:00 2001 From: Adam Nemet Date: Tue, 19 Sep 2017 23:00:55 +0000 Subject: [PATCH] Allow ORE.emit to take a closure to delay building the remark object In the lambda we are now returning the remark by value so we need to preserve its type in the insertion operator. This requires making the insertion operator generic. I've also converted a few cases to use the new API. It seems to work pretty well. See the LoopUnroller for a slightly more interesting case. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@313691 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Analysis/OptimizationDiagnosticInfo.h | 16 ++++- include/llvm/IR/DiagnosticHandler.h | 5 +- include/llvm/IR/DiagnosticInfo.h | 83 ++++++++++++++++++++-- lib/IR/DiagnosticHandler.cpp | 5 ++ lib/IR/DiagnosticInfo.cpp | 16 ++--- lib/Transforms/IPO/Inliner.cpp | 18 +++-- lib/Transforms/Utils/LoopUnroll.cpp | 23 +++--- lib/Transforms/Vectorize/LoopVectorize.cpp | 6 +- 8 files changed, 138 insertions(+), 34 deletions(-) diff --git a/include/llvm/Analysis/OptimizationDiagnosticInfo.h b/include/llvm/Analysis/OptimizationDiagnosticInfo.h index 4c3b0fb949b..aae1b2c1b0c 100644 --- a/include/llvm/Analysis/OptimizationDiagnosticInfo.h +++ b/include/llvm/Analysis/OptimizationDiagnosticInfo.h @@ -24,7 +24,6 @@ namespace llvm { class DebugLoc; -class LLVMContext; class Loop; class Pass; class Twine; @@ -71,6 +70,21 @@ public: /// optimization record file. void emit(DiagnosticInfoOptimizationBase &OptDiag); + /// \brief Take a lambda that returns a remark which will be emitted. Second + /// argument is only used to restrict this to functions. + template + void emit(T RemarkBuilder, decltype(RemarkBuilder()) * = nullptr) { + // Avoid building the remark unless we know there are at least *some* + // remarks enabled. We can't currently check whether remarks are requested + // for the calling pass since that requires actually building the remark. + + if (F->getContext().getDiagnosticsOutputFile() || + F->getContext().getDiagHandlerPtr()->isAnyRemarkEnabled()) { + auto R = RemarkBuilder(); + emit(R); + } + } + /// \brief Whether we allow for extra compile-time budget to perform more /// analysis to produce fewer false positives. /// diff --git a/include/llvm/IR/DiagnosticHandler.h b/include/llvm/IR/DiagnosticHandler.h index c9a606f02f7..9256d4850df 100644 --- a/include/llvm/IR/DiagnosticHandler.h +++ b/include/llvm/IR/DiagnosticHandler.h @@ -60,12 +60,15 @@ struct DiagnosticHandler { /// to provide different implementation. virtual bool isPassedOptRemarkEnabled(StringRef PassName) const; - /// Return true if any type of remarks are enabled. + /// Return true if any type of remarks are enabled for this pass. bool isAnyRemarkEnabled(StringRef PassName) const { return (isMissedOptRemarkEnabled(PassName) || isPassedOptRemarkEnabled(PassName) || isAnalysisRemarkEnabled(PassName)); } + + /// Return true if any type of remarks are enabled for any pass. + virtual bool isAnyRemarkEnabled() const; }; } // namespace llvm diff --git a/include/llvm/IR/DiagnosticInfo.h b/include/llvm/IR/DiagnosticInfo.h index a2023554a89..1e9bcb67e28 100644 --- a/include/llvm/IR/DiagnosticInfo.h +++ b/include/llvm/IR/DiagnosticInfo.h @@ -438,10 +438,10 @@ public: : DiagnosticInfoWithLocationBase(Kind, Severity, Fn, Loc), PassName(PassName), RemarkName(RemarkName) {} - DiagnosticInfoOptimizationBase &operator<<(StringRef S); - DiagnosticInfoOptimizationBase &operator<<(Argument A); - DiagnosticInfoOptimizationBase &operator<<(setIsVerbose V); - DiagnosticInfoOptimizationBase &operator<<(setExtraArgs EA); + void insert(StringRef S); + void insert(Argument A); + void insert(setIsVerbose V); + void insert(setExtraArgs EA); /// \see DiagnosticInfo::print. void print(DiagnosticPrinter &DP) const override; @@ -511,6 +511,81 @@ protected: friend struct yaml::MappingTraits; }; +/// Allow the insertion operator to return the actual remark type rather than a +/// common base class. This allows returning the result of the insertion +/// directly by value, e.g. return OptimizationRemarkAnalysis(...) << "blah". +template +RemarkT & +operator<<(RemarkT &R, + typename std::enable_if< + std::is_base_of::value, + StringRef>::type S) { + R.insert(S); + return R; +} + +/// Also allow r-value for the remark to allow insertion into a +/// temporarily-constructed remark. +template +RemarkT & +operator<<(RemarkT &&R, + typename std::enable_if< + std::is_base_of::value, + StringRef>::type S) { + R.insert(S); + return R; +} + +template +RemarkT & +operator<<(RemarkT &R, + typename std::enable_if< + std::is_base_of::value, + DiagnosticInfoOptimizationBase::Argument>::type A) { + R.insert(A); + return R; +} + +template +RemarkT & +operator<<(RemarkT &&R, + typename std::enable_if< + std::is_base_of::value, + DiagnosticInfoOptimizationBase::Argument>::type A) { + R.insert(A); + return R; +} + +template +RemarkT & +operator<<(RemarkT &R, + typename std::enable_if< + std::is_base_of::value, + DiagnosticInfoOptimizationBase::setIsVerbose>::type V) { + R.insert(V); + return R; +} + +template +RemarkT & +operator<<(RemarkT &&R, + typename std::enable_if< + std::is_base_of::value, + DiagnosticInfoOptimizationBase::setIsVerbose>::type V) { + R.insert(V); + return R; +} + +template +RemarkT & +operator<<(RemarkT &R, + typename std::enable_if< + std::is_base_of::value, + DiagnosticInfoOptimizationBase::setExtraArgs>::type EA) { + R.insert(EA); + return R; +} + /// \brief Common features for diagnostics dealing with optimization remarks /// that are used by IR passes. class DiagnosticInfoIROptimization : public DiagnosticInfoOptimizationBase { diff --git a/lib/IR/DiagnosticHandler.cpp b/lib/IR/DiagnosticHandler.cpp index ad4f5dac8d8..fb1ac438ffb 100644 --- a/lib/IR/DiagnosticHandler.cpp +++ b/lib/IR/DiagnosticHandler.cpp @@ -84,3 +84,8 @@ bool DiagnosticHandler::isPassedOptRemarkEnabled(StringRef PassName) const { return (PassRemarksPassedOptLoc.Pattern && PassRemarksPassedOptLoc.Pattern->match(PassName)); } + +bool DiagnosticHandler::isAnyRemarkEnabled() const { + return (PassRemarksPassedOptLoc.Pattern || PassRemarksMissedOptLoc.Pattern || + PassRemarksAnalysisOptLoc.Pattern); +} diff --git a/lib/IR/DiagnosticInfo.cpp b/lib/IR/DiagnosticInfo.cpp index bb9e52abe53..b033f4d5453 100644 --- a/lib/IR/DiagnosticInfo.cpp +++ b/lib/IR/DiagnosticInfo.cpp @@ -315,28 +315,20 @@ void DiagnosticInfoISelFallback::print(DiagnosticPrinter &DP) const { DP << "Instruction selection used fallback path for " << getFunction(); } -DiagnosticInfoOptimizationBase &DiagnosticInfoOptimizationBase:: -operator<<(StringRef S) { +void DiagnosticInfoOptimizationBase::insert(StringRef S) { Args.emplace_back(S); - return *this; } -DiagnosticInfoOptimizationBase &DiagnosticInfoOptimizationBase:: -operator<<(Argument A) { +void DiagnosticInfoOptimizationBase::insert(Argument A) { Args.push_back(std::move(A)); - return *this; } -DiagnosticInfoOptimizationBase &DiagnosticInfoOptimizationBase:: -operator<<(setIsVerbose V) { +void DiagnosticInfoOptimizationBase::insert(setIsVerbose V) { IsVerbose = true; - return *this; } -DiagnosticInfoOptimizationBase &DiagnosticInfoOptimizationBase:: -operator<<(setExtraArgs EA) { +void DiagnosticInfoOptimizationBase::insert(setExtraArgs EA) { FirstExtraArgIndex = Args.size(); - return *this; } std::string DiagnosticInfoOptimizationBase::getMsg() const { diff --git a/lib/Transforms/IPO/Inliner.cpp b/lib/Transforms/IPO/Inliner.cpp index 3c46913f8a8..4233ba9deec 100644 --- a/lib/Transforms/IPO/Inliner.cpp +++ b/lib/Transforms/IPO/Inliner.cpp @@ -356,10 +356,12 @@ shouldInline(CallSite CS, function_ref GetInlineCost, if (IC.isNever()) { DEBUG(dbgs() << " NOT Inlining: cost=never" << ", Call: " << *CS.getInstruction() << "\n"); - ORE.emit(OptimizationRemarkMissed(DEBUG_TYPE, "NeverInline", Call) + ORE.emit([&]() { + return OptimizationRemarkMissed(DEBUG_TYPE, "NeverInline", Call) << NV("Callee", Callee) << " not inlined into " << NV("Caller", Caller) - << " because it should never be inlined (cost=never)"); + << " because it should never be inlined (cost=never)"; + }); return None; } @@ -367,11 +369,13 @@ shouldInline(CallSite CS, function_ref GetInlineCost, DEBUG(dbgs() << " NOT Inlining: cost=" << IC.getCost() << ", thres=" << IC.getThreshold() << ", Call: " << *CS.getInstruction() << "\n"); - ORE.emit(OptimizationRemarkMissed(DEBUG_TYPE, "TooCostly", Call) + ORE.emit([&]() { + return OptimizationRemarkMissed(DEBUG_TYPE, "TooCostly", Call) << NV("Callee", Callee) << " not inlined into " << NV("Caller", Caller) << " because too costly to inline (cost=" << NV("Cost", IC.getCost()) - << ", threshold=" << NV("Threshold", IC.getThreshold()) << ")"); + << ", threshold=" << NV("Threshold", IC.getThreshold()) << ")"; + }); return None; } @@ -581,12 +585,14 @@ inlineCallsImpl(CallGraphSCC &SCC, CallGraph &CG, << NV("Callee", Callee) << " inlined into " << NV("Caller", Caller) << " with cost=always"); else - ORE.emit(OptimizationRemark(DEBUG_TYPE, "Inlined", DLoc, Block) + ORE.emit([&]() { + return OptimizationRemark(DEBUG_TYPE, "Inlined", DLoc, Block) << NV("Callee", Callee) << " inlined into " << NV("Caller", Caller) << " with cost=" << NV("Cost", OIC->getCost()) << " (threshold=" << NV("Threshold", OIC->getThreshold()) - << ")"); + << ")"; + }); // If inlining this function gave us any new call sites, throw them // onto our worklist to process. They are useful inline candidates. diff --git a/lib/Transforms/Utils/LoopUnroll.cpp b/lib/Transforms/Utils/LoopUnroll.cpp index 7759ac74d56..1fdc5e124e5 100644 --- a/lib/Transforms/Utils/LoopUnroll.cpp +++ b/lib/Transforms/Utils/LoopUnroll.cpp @@ -476,23 +476,30 @@ bool llvm::UnrollLoop(Loop *L, unsigned Count, unsigned TripCount, bool Force, << " peeled loop by " << NV("PeelCount", PeelCount) << " iterations"); } else { - OptimizationRemark Diag(DEBUG_TYPE, "PartialUnrolled", L->getStartLoc(), - L->getHeader()); - Diag << "unrolled loop by a factor of " << NV("UnrollCount", Count); + auto DiagBuilder = [&]() { + OptimizationRemark Diag(DEBUG_TYPE, "PartialUnrolled", L->getStartLoc(), + L->getHeader()); + return Diag << "unrolled loop by a factor of " + << NV("UnrollCount", Count); + }; DEBUG(dbgs() << "UNROLLING loop %" << Header->getName() << " by " << Count); if (TripMultiple == 0 || BreakoutTrip != TripMultiple) { DEBUG(dbgs() << " with a breakout at trip " << BreakoutTrip); - ORE->emit(Diag << " with a breakout at trip " - << NV("BreakoutTrip", BreakoutTrip)); + ORE->emit([&]() { + return DiagBuilder() << " with a breakout at trip " + << NV("BreakoutTrip", BreakoutTrip); + }); } else if (TripMultiple != 1) { DEBUG(dbgs() << " with " << TripMultiple << " trips per branch"); - ORE->emit(Diag << " with " << NV("TripMultiple", TripMultiple) - << " trips per branch"); + ORE->emit([&]() { + return DiagBuilder() << " with " << NV("TripMultiple", TripMultiple) + << " trips per branch"; + }); } else if (RuntimeTripCount) { DEBUG(dbgs() << " with run-time trip count"); - ORE->emit(Diag << " with run-time trip count"); + ORE->emit([&]() { return DiagBuilder() << " with run-time trip count"; }); } DEBUG(dbgs() << "!\n"); } diff --git a/lib/Transforms/Vectorize/LoopVectorize.cpp b/lib/Transforms/Vectorize/LoopVectorize.cpp index d339143bf64..c14ae0d962c 100644 --- a/lib/Transforms/Vectorize/LoopVectorize.cpp +++ b/lib/Transforms/Vectorize/LoopVectorize.cpp @@ -1231,12 +1231,14 @@ public: // FIXME: Add interleave.disable metadata. This will allow // vectorize.disable to be used without disabling the pass and errors // to differentiate between disabled vectorization and a width of 1. - ORE.emit(OptimizationRemarkAnalysis(vectorizeAnalysisPassName(), + ORE.emit([&]() { + return OptimizationRemarkAnalysis(vectorizeAnalysisPassName(), "AllDisabled", L->getStartLoc(), L->getHeader()) << "loop not vectorized: vectorization and interleaving are " "explicitly disabled, or the loop has already been " - "vectorized"); + "vectorized"; + }); return false; } -- 2.11.0