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

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

almost looks good, just a couple of suggestions



================
Comment at: llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp:118
   if (!hasForceAttributes())
     return PreservedAnalyses::all();
 
----------------
we can only return this if we haven't made any changes to the IR.

the simplest way is have a `bool Changed = false;` at the top, set it to true in the `!CSVFilePath.empty()` block, and change the below to
```
if (hasForceAttributes()) {
  for (Function &F : M.functions())
    forceAttributes(F);
  Changed = true;
}
// Just conservatively invalidate analyses if we've made any changes, this isn't likely to be important.
return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
```


================
Comment at: llvm/test/Transforms/ForcedFunctionAttrs/FunctionAnnotator.ll:35
+
+; RUN: opt -passes='forceattrs' -forceattrs-csv-path="%S/DoesNotExist.csv" -S < %s 2>&1 | FileCheck %s --check-prefix=NonexistentFunc
+; NonexistentFunc: Function in CSV file at line 1 does not exist.
----------------
we typically put all RUN lines at the very top


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155617