[clang] 60d3947 - [remark][diagnostics] Using clang diagnostic handler for IR input files

Rong Xu via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 14 15:45:47 PST 2020


Author: Rong Xu
Date: 2020-01-14T15:44:57-08:00
New Revision: 60d39479221d6bc09060f7816bcd7c54eb286603

URL: https://github.com/llvm/llvm-project/commit/60d39479221d6bc09060f7816bcd7c54eb286603
DIFF: https://github.com/llvm/llvm-project/commit/60d39479221d6bc09060f7816bcd7c54eb286603.diff

LOG: [remark][diagnostics] Using clang diagnostic handler for IR input files

For IR input files, we currently use LLVM diagnostic handler even the
compilation is from clang. As a result, we are not able to use -Rpass
to get the transformation reports. Some warnings are not handled
properly either: We found many mysterious warnings in our ThinLTO backend
compilations in SamplePGO and CSPGO. An example of the warning:
"warning: net/proto2/public/metadata_lite.h:51:21: 0.02% (1 / 4999)"

This turns out to be a warning by Wmisexpect, which is supposed to be
filtered out by default. But since the filter is in clang's
diagnostic hander, we emit these incomplete warnings from LLVM's
diagnostic handler.

This patch uses clang diagnostic handler for IR input files. We create
a fake backendconsumer just to install the diagnostic handler.

With this change, we will have proper handling of all the warnings and we can
use -Rpass* options in IR input files compilation.
Also note that with is patch, LLVM's diagnostic options, like
"-mllvm -pass-remarks=*", are no longer be able to get optimization remarks.

Differential Revision: https://reviews.llvm.org/D72523

Added: 
    clang/test/CodeGen/Inputs/thinlto_expect1.proftext
    clang/test/CodeGen/Inputs/thinlto_expect2.proftext
    clang/test/CodeGen/thinlto-clang-diagnostic-handler-in-be.c

Modified: 
    clang/lib/CodeGen/CodeGenAction.cpp
    clang/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 7f3f358d3d98..7065e78f19a2 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -151,6 +151,29 @@ namespace clang {
       FrontendTimesIsEnabled = TimePasses;
       llvm::TimePassesIsEnabled = TimePasses;
     }
+
+    // This constructor is used in installing an empty BackendConsumer
+    // to use the clang diagnostic handler for IR input files. It avoids
+    // initializing the OS field.
+    BackendConsumer(BackendAction Action, DiagnosticsEngine &Diags,
+                    const HeaderSearchOptions &HeaderSearchOpts,
+                    const PreprocessorOptions &PPOpts,
+                    const CodeGenOptions &CodeGenOpts,
+                    const TargetOptions &TargetOpts,
+                    const LangOptions &LangOpts, bool TimePasses,
+                    SmallVector<LinkModule, 4> LinkModules, LLVMContext &C,
+                    CoverageSourceInfo *CoverageInfo = nullptr)
+        : Diags(Diags), Action(Action), HeaderSearchOpts(HeaderSearchOpts),
+          CodeGenOpts(CodeGenOpts), TargetOpts(TargetOpts), LangOpts(LangOpts),
+          Context(nullptr),
+          LLVMIRGeneration("irgen", "LLVM IR Generation Time"),
+          LLVMIRGenerationRefCount(0),
+          Gen(CreateLLVMCodeGen(Diags, "", HeaderSearchOpts, PPOpts,
+                                CodeGenOpts, C, CoverageInfo)),
+          LinkModules(std::move(LinkModules)) {
+      FrontendTimesIsEnabled = TimePasses;
+      llvm::TimePassesIsEnabled = TimePasses;
+    }
     llvm::Module *getModule() const { return Gen->GetModule(); }
     std::unique_ptr<llvm::Module> takeModule() {
       return std::unique_ptr<llvm::Module>(Gen->ReleaseModule());
@@ -616,10 +639,20 @@ void BackendConsumer::UnsupportedDiagHandler(
   StringRef Filename;
   unsigned Line, Column;
   bool BadDebugInfo = false;
-  FullSourceLoc Loc =
-      getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, Line, Column);
+  FullSourceLoc Loc;
+  std::string Msg;
+  raw_string_ostream MsgStream(Msg);
 
-  Diags.Report(Loc, diag::err_fe_backend_unsupported) << D.getMessage().str();
+  // Context will be nullptr for IR input files, we will construct the diag
+  // message from llvm::DiagnosticInfoUnsupported.
+  if (Context != nullptr) {
+    Loc = getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, Line, Column);
+    MsgStream << D.getMessage();
+  } else {
+    DiagnosticPrinterRawOStream DP(MsgStream);
+    D.print(DP);
+  }
+  Diags.Report(Loc, diag::err_fe_backend_unsupported) << MsgStream.str();
 
   if (BadDebugInfo)
     // If we were not able to translate the file:line:col information
@@ -635,10 +668,21 @@ void BackendConsumer::MisExpectDiagHandler(
   StringRef Filename;
   unsigned Line, Column;
   bool BadDebugInfo = false;
-  FullSourceLoc Loc =
-      getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, Line, Column);
+  FullSourceLoc Loc;
+  std::string Msg;
+  raw_string_ostream MsgStream(Msg);
+  DiagnosticPrinterRawOStream DP(MsgStream);
 
-  Diags.Report(Loc, diag::warn_profile_data_misexpect) << D.getMsg().str();
+  // Context will be nullptr for IR input files, we will construct the diag
+  // message from llvm::DiagnosticInfoMisExpect.
+  if (Context != nullptr) {
+    Loc = getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, Line, Column);
+    MsgStream << D.getMsg();
+  } else {
+    DiagnosticPrinterRawOStream DP(MsgStream);
+    D.print(DP);
+  }
+  Diags.Report(Loc, diag::warn_profile_data_misexpect) << MsgStream.str();
 
   if (BadDebugInfo)
     // If we were not able to translate the file:line:col information
@@ -658,12 +702,19 @@ void BackendConsumer::EmitOptimizationMessage(
   StringRef Filename;
   unsigned Line, Column;
   bool BadDebugInfo = false;
-  FullSourceLoc Loc =
-      getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, Line, Column);
-
+  FullSourceLoc Loc;
   std::string Msg;
   raw_string_ostream MsgStream(Msg);
-  MsgStream << D.getMsg();
+
+  // Context will be nullptr for IR input files, we will construct the remark
+  // message from llvm::DiagnosticInfoOptimizationBase.
+  if (Context != nullptr) {
+    Loc = getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, Line, Column);
+    MsgStream << D.getMsg();
+  } else {
+    DiagnosticPrinterRawOStream DP(MsgStream);
+    D.print(DP);
+  }
 
   if (D.getHotness())
     MsgStream << " (hotness: " << *D.getHotness() << ")";
@@ -1088,12 +1139,20 @@ void CodeGenAction::ExecuteAction() {
     Ctx.setInlineAsmDiagnosticHandler(BitcodeInlineAsmDiagHandler,
                                       &Diagnostics);
 
+    // Set clang diagnostic handler. To do this we need to create a fake
+    // BackendConsumer.
+    BackendConsumer Result(BA, CI.getDiagnostics(), CI.getHeaderSearchOpts(),
+                           CI.getPreprocessorOpts(), CI.getCodeGenOpts(),
+                           CI.getTargetOpts(), CI.getLangOpts(),
+                           CI.getFrontendOpts().ShowTimers,
+                           std::move(LinkModules), *VMContext, nullptr);
+    Ctx.setDiagnosticHandler(
+        std::make_unique<ClangDiagnosticHandler>(CodeGenOpts, &Result));
+
     Expected<std::unique_ptr<llvm::ToolOutputFile>> OptRecordFileOrErr =
         setupOptimizationRemarks(
-            Ctx, CodeGenOpts.OptRecordFile,
-            CodeGenOpts.OptRecordPasses,
-            CodeGenOpts.OptRecordFormat,
-            CodeGenOpts.DiagnosticsWithHotness,
+            Ctx, CodeGenOpts.OptRecordFile, CodeGenOpts.OptRecordPasses,
+            CodeGenOpts.OptRecordFormat, CodeGenOpts.DiagnosticsWithHotness,
             CodeGenOpts.DiagnosticsHotnessThreshold);
 
     if (Error E = OptRecordFileOrErr.takeError()) {
@@ -1101,10 +1160,10 @@ void CodeGenAction::ExecuteAction() {
       return;
     }
     std::unique_ptr<llvm::ToolOutputFile> OptRecordFile =
-      std::move(*OptRecordFileOrErr);
+        std::move(*OptRecordFileOrErr);
 
-    EmitBackendOutput(Diagnostics, CI.getHeaderSearchOpts(),
-                      CodeGenOpts, TargetOpts, CI.getLangOpts(),
+    EmitBackendOutput(Diagnostics, CI.getHeaderSearchOpts(), CodeGenOpts,
+                      TargetOpts, CI.getLangOpts(),
                       CI.getTarget().getDataLayout(), TheModule.get(), BA,
                       std::move(OS));
 

diff  --git a/clang/test/CodeGen/Inputs/thinlto_expect1.proftext b/clang/test/CodeGen/Inputs/thinlto_expect1.proftext
new file mode 100644
index 000000000000..e7ce3a4ee237
--- /dev/null
+++ b/clang/test/CodeGen/Inputs/thinlto_expect1.proftext
@@ -0,0 +1,11 @@
+# IR level Instrumentation Flag
+:ir
+foo
+# Func Hash:
+25571299074
+# Num Counters:
+2
+# Counter Values:
+12
+24
+

diff  --git a/clang/test/CodeGen/Inputs/thinlto_expect2.proftext b/clang/test/CodeGen/Inputs/thinlto_expect2.proftext
new file mode 100644
index 000000000000..f9de785587ab
--- /dev/null
+++ b/clang/test/CodeGen/Inputs/thinlto_expect2.proftext
@@ -0,0 +1,20 @@
+# CSIR level Instrumentation Flag
+:csir
+foo
+# Func Hash:
+25571299074
+# Num Counters:
+2
+# Counter Values:
+12
+24
+
+foo
+# Func Hash:
+1152921530178146050
+# Num Counters:
+2
+# Counter Values:
+24
+12
+

diff  --git a/clang/test/CodeGen/thinlto-clang-diagnostic-handler-in-be.c b/clang/test/CodeGen/thinlto-clang-diagnostic-handler-in-be.c
new file mode 100644
index 000000000000..3c410571a90b
--- /dev/null
+++ b/clang/test/CodeGen/thinlto-clang-diagnostic-handler-in-be.c
@@ -0,0 +1,24 @@
+// Test clang diagnositic handler works in IR file compilation.
+
+// REQUIRES: x86-registered-target
+
+// RUN: llvm-profdata merge -o %t1.profdata %S/Inputs/thinlto_expect1.proftext
+// RUN: %clang -O2 -fexperimental-new-pass-manager -flto=thin -g -fprofile-use=%t1.profdata -c -o %t1.bo %s
+// RUN: llvm-lto -thinlto -o %t %t1.bo
+// RUN: %clang -cc1 -O2 -fexperimental-new-pass-manager -x ir %t1.bo -fthinlto-index=%t.thinlto.bc -emit-obj -Rpass-analysis=info 2>&1 | FileCheck %s -check-prefix=CHECK-REMARK
+// RUN: llvm-profdata merge -o %t2.profdata %S/Inputs/thinlto_expect2.proftext
+// RUN: %clang -cc1 -O2 -fexperimental-new-pass-manager -x ir %t1.bo -fthinlto-index=%t.thinlto.bc -fprofile-instrument-use-path=%t2.profdata -emit-obj -Wmisexpect 2>&1 | FileCheck %s -check-prefix=CHECK-WARNING
+// RUN: %clang -cc1 -O2 -fexperimental-new-pass-manager -x ir %t1.bo -fthinlto-index=%t.thinlto.bc -fprofile-instrument-use-path=%t2.profdata -emit-obj 2>&1 | FileCheck %s -allow-empty -check-prefix=CHECK-NOWARNING
+
+int sum;
+__attribute__((noinline)) void bar() {
+  sum = 1234;
+}
+
+__attribute__((noinline)) void foo(int m) {
+  if (__builtin_expect(m > 9, 1))
+    bar();
+}
+// CHECK-REMARK: remark: {{.*}}.c:
+// CHECK-WARNING: warning: Potential performance regression from use of __builtin_expect(): Annotation was correct on {{.*}}.c:{{[0-9]*}}:26: 50.00% (12 / 24) of profiled executions.
+// CHECK-NOWARNING-NOT: warning: {{.*}}.c:{{[0-9]*}}:26: 50.00% (12 / 24)

diff  --git a/clang/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll b/clang/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
index 0362786ecdbf..5033203765c6 100644
--- a/clang/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
+++ b/clang/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
@@ -28,7 +28,7 @@
 ; YAML-NEXT: ...
 
 ; Next try with pass remarks to stderr
-; RUN: %clang -target x86_64-scei-ps4 -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm -pass-remarks=inline -fdiagnostics-show-hotness -o %t2.o -c 2>&1 | FileCheck %s
+; RUN: %clang -target x86_64-scei-ps4 -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -Rpass=inline -fdiagnostics-show-hotness -o %t2.o -c 2>&1 | FileCheck %s
 
 ; CHECK: tinkywinky inlined into main with (cost=0, threshold=337) (hotness: 300)
 


        


More information about the cfe-commits mailing list