const-prop regression for Float^Int

[albheim]

Mentioned this in #45656 as well as a little discussion on discord.

It seems like when doing exponentiation with Float^Int as literals, the compiler used to precalculate that in 1.7, but in 1.8 it does not do that anymore (except for some small values).

So an example that is around 5 times slower in 1.8.0-rc1 compared to 1.7.3 for me is

function f(x)
    A = 2.0^34
    A * x
end

where for 1.7.3 the llvm is simply

julia> @code_llvm debuginfo=:none f(2.0)
define double @julia_f_914(double %0) #0 {
top:
  %1 = fmul double %0, 0x4210000000000000
  ret double %1
}

and for 1.8.0-rc1

define double @julia_f_1270(double %0) #0 {
top:
  %1 = call double @j_pow_body_1272(double 2.000000e+00, i64 signext 34) #0
  %2 = fmul double %1, %0
  ret double %2
}

The changes seems to have been made since it made the calculation faster, but I feel like it should hopefully work to have it both fast and have the compiler able to precalculate on literals?

[martinholters]

--- a/base/math.jl
+++ b/base/math.jl
@@ -42,7 +42,7 @@ end
 
 # non-type specific math functions
 
-@inline function two_mul(x::Float64, y::Float64)
+@assume_effects :consistent @inline function two_mul(x::Float64, y::Float64)
     if Core.Intrinsics.have_fma(Float64)
         xy = x*y
         return xy, fma(x, y, -xy)

seems sufficient and should be valid.

[oscardssmith]

is this not getting marked consistent because have_fma isn't consistent? if so, that's annoying, but makes sense.

[martinholters]

is this not getting marked consistent because have_fma isn't consistent?

I think so, yes. And I'm not quite clear whether it could be. But IIUC, both branches of two_mul should yield equal results, so marking it :consistent should be valid in any case.

[Liozou]

Just a reminder that the docstring of ??Base.@assume_effects states explicitly that:

The :consistent-cy includes all legal rewrites performed by the optimizer. For example, floating-point fastmath operations are not considered :consistent, because the optimizer may rewrite them causing the output to not be :consistent, even for the same world age (e.g. because one ran in the interpreter, while the other was optimized).

so I don't think marking as :consistent any function involving fma is allowed.

[N5N3]

so I don't think marking as :consistent any function involving fma is allowed.

fma itself should be consistent. Do you mean muladd?

[Liozou]

Oh, right. Seems I was confused, sorry for the noise.

[mikmoore]

Can someone answer a technical question for me?

I see the PR #46022 to resolve this issue and it involves a @assume_effects :consistent annotation for two_mul. But, if I understand the above conversation, the reason this is necessary is because Core.Intrinsics.have_fma is inconsistent and so taints the default effects analysis? Why is have_fma inconsistent? On any given machine, shouldn't it always return the same result? Couldn't it be annotated consistent and then we wouldn't need the downstream annotation on two_mul? Is the hazard that this stuff ends up baked into the sys image? It sounds tedious, needing to maintain annotations for the effects of every function depending on have_fma (and similar functions that already or someday might exist).

Barring that, couldn't we check have_fma at load time and then @eval the proper definition, which would then infer the proper effects?

[oscardssmith]

the short answer is that that's not valid, but I don't think it will matter much because have_fma isn't going to be used a lot.