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

change x^p for literal p from x^Val{p} to x^Val{p}() #20783

Closed
wants to merge 1 commit into from

Conversation

stevengj
Copy link
Member

This changes #20530 to use Val{p}() rather than Val{p}, so that we dispatch on ^{p}(x, ::Val{p}) rather than ^{p}(x, ::Type{Val{p}}).

This change was suggested by @vtjnash in #19890 because the old behavior thwarts constant folding. With Val{p}(), constant-folding works again:

julia> f(x) = 2.0^3 * x
f (generic function with 1 method)

julia> @code_llvm f(4.0)

define double @julia_f_67331(double) #0 !dbg !5 {
top:
  %1 = fmul double %0, 8.000000e+00
  ret double %1
}

@stevengj
Copy link
Member Author

(Will have to be rebased after #20782 is merged.)

@stevengj stevengj added the maths Mathematical functions label Feb 24, 2017
@stevengj
Copy link
Member Author

Hmm, looks like constant folding of literal powers works even without this patch. Maybe it was fixed by 0c5eac2?

@vtjnash, do you still think we should change from passing Val{x} to Val{x}()?

@JeffBezanson
Copy link
Member

I don't think it's especially important for ^; it would just be nicer to use Val(x) and Val{x} instead of Val{x} and Type{Val{x}}, everywhere, assuming @vtjnash agrees it's workable.

@musm
Copy link
Contributor

musm commented Feb 24, 2017

with this change is it possible to remove the @eval and interpolation in line https://github.com/JuliaLang/julia/blob/master/base/special/exp.jl#L59 ?

@stevengj
Copy link
Member Author

Okay, then I will close this PR. At some point in the future, maybe there will be a general overhaul of this.

@stevengj stevengj closed this Feb 24, 2017
@stevengj
Copy link
Member Author

@musm, I think it is already possible to remove the @eval. Even with this PR, I get:

julia> foo(::Type{Float64}) = (2.0^-28)
foo (generic function with 1 method)

julia> @code_llvm foo(Float64)

define double @julia_foo_67398(i8**) #0 !dbg !5 {
L13:
  ret double 0x3E30000000000000
}

@StefanKarpinski
Copy link
Member

Even better: wouldn't Val be unnecessary if constant prop improved?

@stevengj
Copy link
Member Author

@StefanKarpinski, if you actually want to dispatch on a value, it doesn't seem like compiler optimizations will help?

@vtjnash
Copy link
Member

vtjnash commented Feb 24, 2017

if you actually want to dispatch on a value

Yes, but we already have these things called function names. It turns out you can even include practically arbitrary strings in them! And it's even way shorter to type f_transposed than f(Val{:transposed}()) :)

Yes, the eval isn't necessary. But since it's at the top-level, it's wrapped in eval anyways, and it might as well make use of that to just insert the intended constant early.

@vtjnash
Copy link
Member

vtjnash commented Feb 24, 2017

it would just be nicer to use Val(x) and Val{x}

It's possible that the dispatch table for Val(x) will end up pretty small (Symbol, Char, Int, and maybe UInt or something like that), and thus end up being mostly pretty efficient. We will also eventually need to ensure that constructors for Type{<primary>} work fairly well, so this seems more likely to get a fast-path in the future. I suspect that dispatch on a Type in the non-initial position (e.g. the current common use of Val) won't be beneficial to optimize.

@stevengj
Copy link
Member Author

Well, there is the case of wanting to dispatch differently for x^-2 (if we decide to make e.g. 2^-2 work) or (for Unitful, x^2 and x^3 etcetera so that the units of the return type can be inferred); you wouldn't use a different function name in those cases.

@RauliRuohonen
Copy link

it would just be nicer to use Val(x) and Val{x} instead of Val{x} and Type{Val{x}}, everywhere

The docs (src/manual/types.md) currently say:

For consistency across Julia, the call site should always pass a Valtype rather than creating
an instance, i.e., use foo(Val{:bar}) rather than foo(Val{:bar}()).

If this has changed to the opposite, updating the docs would be good.

@JeffBezanson
Copy link
Member

I said it would be nicer, i.e. objecting to the current state. It has not changed. If we changed it, we'd update the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maths Mathematical functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants