-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[FnSpecialization] Enable function specialization of call chains #163891
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
base: main
Are you sure you want to change the base?
Conversation
…han the minimum amount If the knob for minimum code size is turned down low enough, for small functions: `MinCodeSizeSavings * FuncSize / 100` will evaluate to `0`, and then with strict less than we will accept Specialization that doesn't lead to any benefit.
…ucture The data structure will eventually contain extra data for chained and indirect specialization.
…ogic to its own function Will want to call recursively for chains.
…ization into macro Will need to call recursively. No functional change.
Spec contains a Function, and will need to pass extra information with Chaining.
Used to be a single object within findSpecializations() since each Function only entered findSpecializations() once. But will now be going in arbitrary order with Chains.
…zation Cannot rely on AllSpecs to be inorder after Chaining.
If a function is called with constants that passes those constants to another function, try to specialize both of those functions.
…en only ever part of a chain Will get specialized as part of the chain if the chain scores well enough.
… chains in NSpecs Will get specialized as part of chain, so aren't viable as a standalone.
When calculating possible Chains, use the metrics saved as part of the sub-specializations.
…ed functions Otherwise confusing with Chaining.
In the future we won't know the Function at the time of insertion, so need to store and index so we can look up the Argument later.
…of arguments, skip chaining See test/Transforms/FunctionSpecialization/compiler-crash-60191.ll
…posed by specialization
…s part of a chain This way we can still more accurately see the effect of the specialization.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
The function |
Agreed, thank you for the correction! My understanding is that The example on Godbolt only specializes on the current upstream due to some funny behavior of the function specializer, see #164867 (this change is included in this MR as well, I have begun splitting off small chunks that can stand on their own). |
|
I briefly looked at the patch series and I am not convinced it's the right approach. Perhaps it's best to handle non constant foldable calls in the instruction cost visitor separately, similarly to branches and switches which are not folded to a constant. I mean to compute the profitability of specializing that call instead of folding it. But then all this adds compile time complexity which I am not sure it is worth it. |
|
Sorry for the slow response, and thanks for taking the time to engage.
I think that is a competitive approach, here were my pro's/con's between the two approaches:
Pro's of the approach you suggested:
We were looking into this for a particular case in |
Currently, we don't optimize function-specialize cases like the following:
because when specializing for the
3passed tobar(), the value isn't propagated intofoo()to gauge benefit from specializingfoo()as well. With this patch, the optimized code would be:This patch required a fair amount of refactoring before making my changes. That refactoring was done as a series of commits before the main changes for this patch. I'm assuming that (if accepted) this should get merged in as a series of MRs.
The series of commits
(x/6)all belong together since they are the same functional change, but I left them as separate commits for easier reading.At a high level, each
Specelement can haveSubSpecswhich are functions that it forwards its constant argument(s) to and should be specialized along with it. In the above exampled, theSpecforbar(3)would have aSubSpecforfoo(3).