anemet added a comment.
Only minor things at this point. This is very close now.
================
Comment at: include/llvm/Analysis/OptimizationDiagnosticInfo.h:81
/// detected by the user.
- bool allowExtraAnalysis() const {
- // For now, only allow this with -fsave-optimization-record since the
-Rpass
- // options are handled in the front-end.
- return F->getContext().getDiagnosticsOutputFile();
+ bool allowExtraAnalysis(StringRef &&PassName) const {
+ return (F->getContext().getDiagnosticsOutputFile() ||
----------------
Why rvalue reference, you should just take this by value. There is no
ownership transfer here.
================
Comment at: include/llvm/Analysis/OptimizationDiagnosticInfo.h:84
+ F->getContext().getDiagHandler()->isAnyRemarkEnabled(
+ std::move(PassName)));
}
----------------
No std::move here. You have more of this later.
================
Comment at: include/llvm/IR/DiagnosticHandler.h:13
+
+#include "llvm/Support/Regex.h"
+
----------------
I don't think you need this.
================
Comment at: include/llvm/IR/DiagnosticHandler.h:18
+
+struct DiagnosticHandler {
+public:
----------------
Please add a comment before the class.
================
Comment at: include/llvm/IR/DiagnosticHandler.h:19
+struct DiagnosticHandler {
+public:
+ void *DiagnosticContext = nullptr;
----------------
No need for public for struct.
================
Comment at: include/llvm/IR/DiagnosticHandler.h:27
+
+ /// DiagHandlerCallback is settable from the C API and base implimentation
+ /// of DiagnosticHandler will call it from handleDiagnostics(). Any derived
----------------
implementation
================
Comment at: include/llvm/IR/DiagnosticHandler.h:45
+
+ /// checks if remark requested with -pass-remarks-analysis, override
+ /// to provide different implementation
----------------
Capitalize first word and end with period; comments are full sentences.
================
Comment at: include/llvm/IR/DiagnosticHandler.h:57
+
+ /// checks if remark requested with -pass-remarks{-missed/-analysis}
+ bool isAnyRemarkEnabled(StringRef &&PassName) const {
----------------
I would drop the flag names here since if those are overridden this is not
true. Just say "Return true if any remark type is enabled."
================
Comment at: lib/IR/DiagnosticInfo.cpp:47-65
/// \brief Regular expression corresponding to the value given in one of the
/// -pass-remarks* command line flags. Passes whose name matches this regexp
/// will emit a diagnostic via ORE->emit(...);
struct PassRemarksOpt {
std::shared_ptr<Regex> Pattern;
void operator=(const std::string &Val) {
----------------
Is this still used here?
================
Comment at: lib/LTO/LTOCodeGenerator.cpp:667-668
- return Context.setDiagnosticHandler(nullptr, nullptr);
- // Register the LTOCodeGenerator stub in the LLVMContext to forward the
- // diagnostic to the external DiagHandler.
- Context.setDiagnosticHandler(LTOCodeGenerator::DiagnosticHandler, this,
----------------
I think that this comment still applies.
================
Comment at: tools/llvm-lto/llvm-lto.cpp:39-72
+static std::string CurrentActivity;
+namespace {
+struct LLVMLTODiagnosticHandler : public DiagnosticHandler {
+ bool handleDiagnostics(const DiagnosticInfo &DI) override {
+ raw_ostream &OS = errs();
+ OS << "llvm-lto: ";
+ switch (DI.getSeverity()) {
----------------
Don't move this code unless you have to. The diff is easier to read that way.
https://reviews.llvm.org/D33514
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
