[clang] c0bc461 - [Clang] Give error message for invalid profile path when compiling IR

[Aiden Grossman via cfe-commits]

Author: Aiden Grossman
Date: 2022-09-16T19:45:57Z
New Revision: c0bc461999fdac918dd26867947c24eb6235c8d0

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

LOG: [Clang] Give error message for invalid profile path when compiling IR

Before this patch, when compiling an IR file (eg the .llvmbc section
from an object file compiled with -Xclang -fembed-bitcode=all) and
profile data was passed in using the -fprofile-instrument-use-path
flag, there would be no error printed (as the previous implementation
relied on the error getting caught again in the constructor of
CodeGenModule which isn't called when -x ir is set). This patch
moves the error checking directly to where the error is caught
originally rather than failing silently in setPGOUseInstrumentor and
waiting to catch it in CodeGenModule to print diagnostic information to
the user.

Regression test added.

Reviewed By: xur, mtrofin

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

Added: 
    clang/test/Profile/profile-does-not-exist-ir.c

Modified: 
    clang/lib/CodeGen/CodeGenModule.cpp
    clang/lib/Frontend/CompilerInvocation.cpp
    clang/test/Profile/profile-does-not-exist.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index bbd42956a4b0..06ad67a492a1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -182,15 +182,11 @@ CodeGenModule::CodeGenModule(ASTContext &C,
   if (CodeGenOpts.hasProfileClangUse()) {
     auto ReaderOrErr = llvm::IndexedInstrProfReader::create(
         CodeGenOpts.ProfileInstrumentUsePath, CodeGenOpts.ProfileRemappingFile);
-    if (auto E = ReaderOrErr.takeError()) {
-      unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
-                                              "Could not read profile %0: %1");
-      llvm::handleAllErrors(std::move(E), [&](const llvm::ErrorInfoBase &EI) {
-        getDiags().Report(DiagID) << CodeGenOpts.ProfileInstrumentUsePath
-                                  << EI.message();
-      });
-    } else
-      PGOReader = std::move(ReaderOrErr.get());
+    // We're checking for profile read errors in CompilerInvocation, so if
+    // there was an error it should've already been caught. If it hasn't been
+    // somehow, trip an assertion.
+    assert(ReaderOrErr);
+    PGOReader = std::move(ReaderOrErr.get());
   }
 
   // If coverage mapping generation is enabled, create the

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index f42bf3058d1a..9f9241054b1e 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1293,12 +1293,15 @@ static std::string serializeXRayInstrumentationBundle(const XRayInstrSet &S) {
 
 // Set the profile kind using fprofile-instrument-use-path.
 static void setPGOUseInstrumentor(CodeGenOptions &Opts,
-                                  const Twine &ProfileName) {
+                                  const Twine &ProfileName,
+                                  DiagnosticsEngine &Diags) {
   auto ReaderOrErr = llvm::IndexedInstrProfReader::create(ProfileName);
-  // In error, return silently and let Clang PGOUse report the error message.
   if (auto E = ReaderOrErr.takeError()) {
-    llvm::consumeError(std::move(E));
-    Opts.setProfileUse(CodeGenOptions::ProfileClangInstr);
+    unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
+                                            "Error in reading profile %0: %1");
+    llvm::handleAllErrors(std::move(E), [&](const llvm::ErrorInfoBase &EI) {
+      Diags.Report(DiagID) << ProfileName.str() << EI.message();
+    });
     return;
   }
   std::unique_ptr<llvm::IndexedInstrProfReader> PGOReader =
@@ -1712,7 +1715,7 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
   }
 
   if (!Opts.ProfileInstrumentUsePath.empty())
-    setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath);
+    setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath, Diags);
 
   if (const Arg *A = Args.getLastArg(OPT_ftime_report, OPT_ftime_report_EQ)) {
     Opts.TimePasses = true;

diff  --git a/clang/test/Profile/profile-does-not-exist-ir.c b/clang/test/Profile/profile-does-not-exist-ir.c
new file mode 100644
index 000000000000..842a3d44a387
--- /dev/null
+++ b/clang/test/Profile/profile-does-not-exist-ir.c
@@ -0,0 +1,4 @@
+// RUN: not %clang_cc1 -emit-llvm -x ir %s -o - -fprofile-instrument-use-path=%t.nonexistent.profdata 2>&1 | FileCheck %s
+
+// CHECK: error: Error in reading profile {{.*}}.nonexistent.profdata:
+// CHECK-NOT: Assertion failed

diff  --git a/clang/test/Profile/profile-does-not-exist.c b/clang/test/Profile/profile-does-not-exist.c
index 5725f76eb80c..88d55d8668ef 100644
--- a/clang/test/Profile/profile-does-not-exist.c
+++ b/clang/test/Profile/profile-does-not-exist.c
@@ -1,4 +1,4 @@
 // RUN: not %clang_cc1 -emit-llvm %s -o - -fprofile-instrument-use-path=%t.nonexistent.profdata 2>&1 | FileCheck %s
 
-// CHECK: error: Could not read profile {{.*}}.nonexistent.profdata:
+// CHECK: error: Error in reading profile {{.*}}.nonexistent.profdata:
 // CHECK-NOT: Assertion failed