[PATCH] D141077: [MemProf] Context disambiguation cloning pass [patch 3/3]

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson marked an inline comment as done.
tejohnson added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp:3152
+        errorOrToExpected(MemoryBuffer::getFile(MemProfImportSummary)));
+    if (Expected<std::unique_ptr<ModuleSummaryIndex>> SummaryOrErr =
+            getModuleSummaryIndex(*ReadSummaryFile)) {
----------------
snehasish wrote:
> I think this will silently skip the if condition on error since the bool operation for the condition will set the error as checked [1]. Wondering if this is intended since the previous line exits on error.
> 
> [1] https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/Error.h#L234
Yes this is a good point, we should also exit if it isn't parseable as a ModuleSummaryIndex. Modified this to ExitOnErr. Will update the patch with this change now, but then will be moving this code out as it is ThinLTO backend specific.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141077