[PATCH] D155617: Pass to annotate functions with appropriate optimization level.

[Puneeth via Phabricator via llvm-commits]

Puneeth-A-R marked 2 inline comments as done.
Puneeth-A-R added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp:115
+        } else {
+          errs() << "Function in CSV file at line " << It.line_number()
+                 << " does not exist\n";
----------------
mtrofin wrote:
> Nit: it is more readable, I think, if you did this:
> 
> ```
> for(++It... etc)
>   auto SplitPair = ..
>   if (SplitPair.second.empty()) {
>     errs() << ...
>     break;
>   }
>   ...
> ```
> 
> because you don't need an `else` (since you break), and thus less indentation. 
> 
> Also `break` because... why bother continuing. It's an error anyway.
Well, the `if (!SplitPair.second.empty()) {` does not have a corresponding `else`. I think, the `else` that you are referring to actually reports the error if the function name is present in the CSV file, but is not found in the module's IR. Because, if the function doesn't exist in the IR, adding any attributes to it would not be possible. But the upcoming lines in the CSV might have a correct function name. So that's what the `continue` does on the next line. We skip the current iteration and move on to the next.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155617