Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LTO] Enable adding custom pass instrumentation callbacks #71268

Closed
wants to merge 1 commit into from

Conversation

igorkudrin
Copy link
Collaborator

The hook allows the calling code to register instrumentation callbacks for the LTO optimization pipeline. In particular, a custom pass filter can be added to skip certain passes from the default pipeline.

@igorkudrin igorkudrin added the LTO Link time optimization (regular/full LTO or ThinLTO) label Nov 4, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 4, 2023

@llvm/pr-subscribers-lto

Author: Igor Kudrin (igorkudrin)

Changes

The hook allows the calling code to register instrumentation callbacks for the LTO optimization pipeline. In particular, a custom pass filter can be added to skip certain passes from the default pipeline.


Full diff: https://github.com/llvm/llvm-project/pull/71268.diff

2 Files Affected:

  • (modified) llvm/include/llvm/LTO/Config.h (+5)
  • (modified) llvm/lib/LTO/LTOBackend.cpp (+2)
diff --git a/llvm/include/llvm/LTO/Config.h b/llvm/include/llvm/LTO/Config.h
index 6fb55f1cf1686a5..fd22c17bc12a38c 100644
--- a/llvm/include/llvm/LTO/Config.h
+++ b/llvm/include/llvm/LTO/Config.h
@@ -257,6 +257,11 @@ struct Config {
       const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols)>;
   CombinedIndexHookFn CombinedIndexHook;
 
+  /// This hook is called when the optimization pipeline is being built.
+  using CustomizeOptPipelineHookFn =
+      std::function<void(PassInstrumentationCallbacks &)>;
+  CustomizeOptPipelineHookFn CustomizeOptPipeline;
+
   /// This is a convenience function that configures this Config object to write
   /// temporary files named after the given OutputFileName for each of the LTO
   /// phases to disk. A client can use this function to implement -save-temps.
diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index ccc4276e36dacf0..7e1b35319a71d59 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -265,6 +265,8 @@ static void runNewPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM,
   ModuleAnalysisManager MAM;
 
   PassInstrumentationCallbacks PIC;
+  if (Conf.CustomizeOptPipeline)
+    Conf.CustomizeOptPipeline(PIC);
   StandardInstrumentations SI(Mod.getContext(), Conf.DebugPassManager,
                               Conf.VerifyEach);
   SI.registerCallbacks(PIC, &MAM);

@igorkudrin
Copy link
Collaborator Author

Ping

@teresajohnson
Copy link
Contributor

Can you add a test, which looks like it will also require adding an LLVM mechanism for setting this hook.

@aeubanks
Copy link
Contributor

the commit message is confusing, this isn't customizing the optimization pipeline, it's customizing PassInstrumentationCallbacks

@aeubanks
Copy link
Contributor

I don't think we have this sort of functionality elsewhere for the default optimization pipelines, why do you specifically just want this for the LTO pipeline?

@igorkudrin igorkudrin changed the title [LTO] Add a hook to customize the optimization pipeline [LTO] Enable adding custom pass instrumentation callbacks Nov 22, 2023
@igorkudrin
Copy link
Collaborator Author

Can you add a test, which looks like it will also require adding an LLVM mechanism for setting this hook.

Added a unittest that uses the hook.

the commit message is confusing, this isn't customizing the optimization pipeline, it's customizing PassInstrumentationCallbacks

Updated. Is it better now?

I don't think we have this sort of functionality elsewhere for the default optimization pipelines, why do you specifically just want this for the LTO pipeline?

We are working on a downstream tool that pre-links bitcode files, runs optimization passes, and produces bitcode file output that can be used later for a final link with LTO, so we run LTO for input files but interrupt the process before codegen. Some passes in the default optimization pipeline, like EliminateAvailableExternallyPass, might be detrimental to this scheme and should be excluded. I suppose the easiest way to accomplish this is to provide a hook that enables adding corresponding filters. I do not think a more complex and general mechanism for customizing the pipeline is needed yet.

The hook allows the calling code to register instrumentation callbacks
for the LTO optimization pipeline. In particular, a custom pass filter
can be added to skip certain passes from the default pipeline.
@teresajohnson
Copy link
Contributor

We are working on a downstream tool that pre-links bitcode files, runs optimization passes, and produces bitcode file output that can be used later for a final link with LTO, so we run LTO for input files but interrupt the process before codegen. Some passes in the default optimization pipeline, like EliminateAvailableExternallyPass, might be detrimental to this scheme and should be excluded. I suppose the easiest way to accomplish this is to provide a hook that enables adding corresponding filters. I do not think a more complex and general mechanism for customizing the pipeline is needed yet.

Interesting, essentially you are doing 2 rounds of LTO?

Is the EliminateAvailableExternallyPass the only one you want to disable? Another approach would be to add an internal option to enable or skip that pass, like many of the other -enable-* flags in PassBuilderPipelines.cpp.

There are existing options to register a custom opt pipeline completely, but I suppose you want something very close to an existing pass pipeline minus a handful of passes?

@igorkudrin
Copy link
Collaborator Author

Interesting, essentially you are doing 2 rounds of LTO?

That's right.

Is the EliminateAvailableExternallyPass the only one you want to disable?

EliminateAvailableExternallyPass is the only pass we have found issues with so far.

Another approach would be to add an internal option to enable or skip that pass, like many of the other -enable-* flags in PassBuilderPipelines.cpp.

These are command-line options and their variables are local to the TU. There is no API to access them from other components. Designing a global filter could be a solution, but for now, it seems too complex for our needs. Moreover, using such an API would mean that components communicate through a global state, which is usually not considered a good program design.

There are existing options to register a custom opt pipeline completely, but I suppose you want something very close to an existing pass pipeline minus a handful of passes?

That's right again. We want to run the typical LTO pass pipelines with only minor customizations.

@teresajohnson
Copy link
Contributor

These are command-line options and their variables are local to the TU. There is no API to access them from other components. Designing a global filter could be a solution, but for now, it seems too complex for our needs. Moreover, using such an API would mean that components communicate through a global state, which is usually not considered a good program design.

I'm not sure what you mean here. How are you invoking your LTO link? You can pass internal options via the linker or other tools that are invoking LTO. E.g. for an lld LTO link: -Wl,-mllvm,-enable-elim-avail-extern=false (making up a possible name for the internal option).

@igorkudrin
Copy link
Collaborator Author

I'm not sure what you mean here. How are you invoking your LTO link? You can pass internal options via the linker or other tools that are invoking LTO. E.g. for an lld LTO link: -Wl,-mllvm,-enable-elim-avail-extern=false (making up a possible name for the internal option).

Our tool uses the LLVM libraries directly, similar to llvm-lto2 and lld, i.e. we create an LTO object, add InputFiles, run the process, and so on, and this is just one of the transformations the tool does. It is not a script or anything like that.

@teresajohnson
Copy link
Contributor

Our tool uses the LLVM libraries directly, similar to llvm-lto2 and lld, i.e. we create an LTO object, add InputFiles, run the process, and so on, and this is just one of the transformations the tool does. It is not a script or anything like that.

Can you clarify what you meant by "These are command-line options and their variables are local to the TU. There is no API to access them from other components." Because if you have a tool that is built like llvm-lto2 etc using LLVM libraries, you should be able to parse the internal command line options passed to it. I'm not sure what is TU local about them?

@igorkudrin
Copy link
Collaborator Author

igorkudrin commented Nov 28, 2023

Can you clarify what you meant by "These are command-line options and their variables are local to the TU. There is no API to access them from other components." Because if you have a tool that is built like llvm-lto2 etc using LLVM libraries, you should be able to parse the internal command line options passed to it. I'm not sure what is TU local about them?

Let's take -enable-npm-synthetic-counts as an example:

static cl::opt<bool> EnableSyntheticCounts("enable-npm-synthetic-counts", ...);

This is a command line option, so a user may be able to specify it on the command line when running the tool. This is suitable for developer-level tools like opt, llc, etc., but does not work well for a user-level tool that has its own options and hides the implementation details. As for our tool, we should not rely on a user to specify the option; moreover, we disable internal options with cl::HideUnrelatedOptions().

The variable for this option is static, so only PassBuilderPipelines.cpp can directly access it.

If we make the variable external, or provide a function that can be called from another translation unit to set the value of the option, this would mean that we are trying to communicate with the PassBuilder through a global state, which is hardly a good design choice.

@teresajohnson
Copy link
Contributor

Ah got it. The typical way then would be to have a flag in the PipelineTuningOptions struct that can be set from your tool and queried by the PM builder.

@igorkudrin
Copy link
Collaborator Author

igorkudrin commented Nov 28, 2023

Ah got it. The typical way then would be to have a flag in the PipelineTuningOptions struct that can be set from your tool and queried by the PM builder.

A setting in the PipelineTuningOptions would be a good variant, especially because the unwanted pass would not even be instantiated and added to the pipeline. However, a flag like EnableEliminateAvailableExternallyPass seems like too ad hoc a solution, and I'd prefer to have a bit more general approach, as there may be other passes in the future that we (or others) may want to disable for a specific task.

@teresajohnson
Copy link
Contributor

Sure, that makes sense. I will let @aeubanks comment more on the original approach (or any other ideas) now that you have clarified the problem.

@igorkudrin
Copy link
Collaborator Author

Gently ping

@aeubanks
Copy link
Contributor

This seems ok, but I feel like what you really want is PB.buildThinLTOPreLinkDefaultPipeline(OL) here.

IIUC, you want optimizations, but more like the ThinLTO pre-link phase (e.g. we don't run EliminateAvailableExternallyPass for the ThinLTO pre-link pipeline for basically the same reasons you want to skip it) that don't drop information. And apparently there's already precedent for something like this with Conf.UseDefaultPipeline. Thoughts on that approach instead?

@igorkudrin
Copy link
Collaborator Author

This seems ok, but I feel like what you really want is PB.buildThinLTOPreLinkDefaultPipeline(OL) here.

Thank you for the idea. The pre-link pipelines seem to suit our needs well. I've prepared another PR for that, #82585.

@igorkudrin igorkudrin closed this Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LTO Link time optimization (regular/full LTO or ThinLTO)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants