[PATCH] D70107: [VFABI] TargetLibraryInfo mappings in IR.

[Andrzej Warzynski via Phabricator via llvm-commits]

andwar added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Utils/InjectTLIMappings.h:22
+  static StringRef name() { return "InjectTLIMappings"; }
+  explicit InjectTLIMappings() {}
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
----------------
jdoerfert wrote:
> I doubt you need the constructor or the name function.
What about this one? You don't need the `name` function, the one provided by the base class is fine: https://github.com/llvm/llvm-project/blob/848007cfbc7509543c5b8604ae063bb6c8ffa0a9/llvm/include/llvm/IR/PassManager.h#L373

As for the constructor, the auto-generated one should be sufficient here. Is it not?


================
Comment at: llvm/lib/Transforms/Utils/InjectTLIMappings.cpp:165
+////////////////////////////////////////////////////////////////////////////////
+// Legacy pass Implementation
+////////////////////////////////////////////////////////////////////////////////
----------------
[nit] ``Legacy PM implementation` (the pass manager is `legacy`, not the pass :) ).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70107