minimal inlining of x^Val{p} for p=0,1,2,3 and x::Number #20648
Conversation
|
@nanosoldier |
…void ambiguities for code that dispatches on the first argument
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
|
Running the benchmarks one more time to see whether the two regressions were noise: @nanosoldier |
| @@ -76,6 +76,8 @@ Language changes | |||
| * Experimental feature: `x^n` for integer literals `n` (e.g. `x^3` | |||
| or `x^-3`) is now lowered to `x^Val{n}`, to enable compile-time | |||
| specialization for literal integer exponents ([#20530]). | |||
| `x^p` for `x::Number` and a literal `p=0,1,2,3` is now lowered to | |||
There was a problem hiding this comment.
p=0,1,2,3,...
edit: when first read, i thought these four numbers are specialized.
There was a problem hiding this comment.
Yes, only these four exponents are inlined by this patch. It's a minimal PR to match (and slightly exceed) what inference.jl did before. In the future, we may well want to inline more exponents.
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
|
Also want to run the benchmarks against the commit before #20530: @nanosoldier |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
|
Is the array comprehension slowdown legitimate here or an artifact of some other change? |
|
I've been seeing that in other recent benchmarks reports. I think it's noise, since it looked like the generated code was identical. |
|
If it's noise, is this okay to merge? |
| @@ -504,6 +504,9 @@ end | |||
| ^(x::BigFloat, y::Integer) = typemin(Clong) <= y <= typemax(Clong) ? x^Clong(y) : x^BigInt(y) | |||
| ^(x::BigFloat, y::Unsigned) = typemin(Culong) <= y <= typemax(Culong) ? x^Culong(y) : x^BigInt(y) | |||
|
|
|||
| # override default inlining of x^2 etc. | |||
| ^{p}(x::BigFloat, ::Type{Val{p}}) = x^Culong(p) | |||
There was a problem hiding this comment.
this gives an InexactError for negative powers, such as BigFloat(2) ^ -20 (which was amazingly not tested)
(wrong line beforehand)
There was a problem hiding this comment.
Should be fixed by #20732
* run NEWS-update.jl #20648 was partially reverted by #20782 * Fix doctest line numbers * Upgrade Documenter, add linkcheck exception bugs.kde.org fails to load from nanosoldier for some reason, gives a CURLE_RECV_ERROR (56) Failure with receiving network data fix alloc.c dead link in devdocs/init.md * fix timeit_init macro in test/perf/perfutil.jl needed to escape its arguments
This PR does very basic inlining of
x^Val{p}forp=0,1,2,3andx::Number(a much more restrictive version of #20637).It should fix the performance regression of #20530.
It also dispatches
^(x,p) = internal_pow(x,p)and definesinternal_pow(x, Val{p})as suggested by @timholy, in order to avoid dispatch ambiguities for code that specializes^on the first argument.