[PATCH] D70691: Optimization record for bytecode input missing- PR44000

Francis Visoiu Mistrih via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 2 08:52:53 PST 2019


thegameg added a comment.

Thanks for fixing this!



================
Comment at: clang/lib/CodeGen/CodeGenAction.cpp:97
+        CGOpts.DiagnosticsHotnessThreshold);
+    if (Error E = OptRecordFileOrErr.takeError()) {
+      handleAllErrors(
----------------
I think what would be nicer here is to extract all the error handling in a separate function and keep using `setupOptimizationRemarks` for all the callers.


================
Comment at: clang/lib/CodeGen/CodeGenAction.cpp:305
+      std::unique_ptr<llvm::ToolOutputFile> OptRecordFile;
+      bool isSet = setOptRecordFile(CodeGenOpts, Diags, Ctx, OptRecordFile);
+
----------------
This would become something like:

```
Expected<std::unique_ptr<llvm::ToolOutputFile>> OptRecordFileOrErr =
      llvm::setupOptimizationRemarks(...);
if (Error E = OptRecordFileOrErr.takeError()) {
  reportError(std::move(E));
  return;
}
```


================
Comment at: clang/test/CodeGen/opt-record-1.c:3
+// RUN: %clang_cc1 -x ir %t.bc -opt-record-file %t.opt -fopenmp -emit-obj
+// RUN: cat %t.opt |  FileCheck -check-prefix=CHECK -check-prefix=CHECK-PASSES %s
+
----------------
Why are the two `-check-prefix`es needed?


================
Comment at: clang/test/CodeGen/opt-record-1.c:4
+// RUN: cat %t.opt |  FileCheck -check-prefix=CHECK -check-prefix=CHECK-PASSES %s
+
+
----------------
Extra blank line here.


================
Comment at: clang/test/CodeGen/opt-record-1.c:18
+// CHECK-PASSES: Pass: inline
+
----------------
Extra blank line here too.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70691/new/

https://reviews.llvm.org/D70691





More information about the llvm-commits mailing list