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

[Andrzej Warzynski via Phabricator via llvm-commits]

andwar added a comment.

IIUC, this a transformation pass (it does modify the module, e.g. by `appendToCompilerUsed(*M, {Global});`). So you probably want to register it with one of the optimisation pipelines. I _believe_ that that's how you do it:

For legacy PM:

- Use `INITIALIZE_PASS_BEGIN` (https://github.com/llvm/llvm-project/blob/848007cfbc7509543c5b8604ae063bb6c8ffa0a9/llvm/include/llvm/PassSupport.h#L33) that will define `initializeInjectTLIMappingsPass` for you. Then use it where you want to add it - you can check `initializeSLPVectorizerPass` for reference (https://github.com/llvm/llvm-project/blob/848007cfbc7509543c5b8604ae063bb6c8ffa0a9/llvm/lib/Transforms/Vectorize/Vectorize.cpp#L28).
- You can also use `llvm::RegisterStandardPasses` (http://llvm.org/docs/WritingAnLLVMPass.html#basic-code-required) if you want to add it in one of the avaiable extension points (so that it runs automagically with e.g. `-O1`)

For the new PM, you probably want to add your pass to an existing `FunctionPassPamanager`, e.g.

- `OptimizePM` (https://github.com/llvm/llvm-project/blob/848007cfbc7509543c5b8604ae063bb6c8ffa0a9/llvm/lib/Passes/PassBuilder.cpp#L948)

Once that's done, your Pass will be run _automagically_ together with other passes in the pipeline. This is just a quick brain-dump so please ping me if it's unclear.



================
Comment at: llvm/include/llvm/Transforms/Utils/InjectTLIMappings.h:19
+namespace llvm {
+class InjectTLIMappingsPass : public PassInfoMixin<InjectTLIMappingsPass> {
+  const TargetLibraryInfo &TLI;
----------------
sdesmalen wrote:
> Don't forget about the old pass manager :)
`Legacy` :)

Also, I am not a fan of appending `Pass` to pass classes (it's clear what this class inherits from either way). Also, if you use `INITIALIZE_PASS_BEGIN`, `Pass` is going to be  prepended to the _Initialize_ method anyway (so you will have `initializeInjestTLIMappingPassPass`): https://github.com/llvm/llvm-project/blob/848007cfbc7509543c5b8604ae063bb6c8ffa0a9/llvm/include/llvm/PassSupport.h#L62


================
Comment at: llvm/lib/Transforms/Utils/InjectTLIMappings.cpp:104
+  const std::string ScalarName = CI->getCalledFunction()->getName();
+  // Nothing to be done if the TLI things the function is not
+  // vectorizable.
----------------
[nit] 'things' -> 'thinks'


================
Comment at: llvm/lib/Transforms/Utils/InjectTLIMappings.cpp:127
+////////////////////////////////////////////////////////////////////////////////
+// Pass declaration and initialization
+////////////////////////////////////////////////////////////////////////////////
----------------
[nit] What follows is the definition of `InjectTLIMappingsPass::run` though.


================
Comment at: llvm/test/Transforms/Util/add-TLI-mappings.ll:1
+; RUN: opt -vector-library=SVML -inject-TLI-mappings -S < %s | FileCheck %s
+
----------------
For this to work you need to register a command line option. Why not use `print-after` and `print-before` instead? Or maybe we do need a command line option?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70107