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

inline native Julia scalars encountered in f.(args...) AST #18205

Merged
merged 1 commit into from
Aug 24, 2016

Conversation

stevengj
Copy link
Member

This is an alternative version of #18202, following a suggestion by @JeffBezanson.

As an optimization, the "dot call" fusion does "inlining" of literal scalars, so that e.g. f.(x,3) turns into broadcast(x -> f(x,3), x) rather than broadcast(f, x, 3). This PR expands the range of scalars that are inlined to include Julia Number objects in the AST (and could do the same for String and some other types once #16966 is fixed … since it is just an optimization, it is okay if we aren't exhaustive).

(As was observed in #18176, the parser actually ends up converting things like floating-point literals into Julia objects in the AST, so without this optimization they aren't inlined.)

@stevengj stevengj added the broadcast Applying a function over a collection label Aug 23, 2016
@JeffBezanson
Copy link
Member

The symbol lookup dance can be avoided by adding the function here instead:

julia/src/ast.c

Line 194 in 2390055

static const builtinspec_t julia_flisp_ast_ext[] = {

I should have thought of that. Sorry to cause you this trouble.

@stevengj
Copy link
Member Author

Ah, thanks for the tip.

@stevengj
Copy link
Member Author

Okay, fixed; now it's a lot simpler.

@JeffBezanson JeffBezanson merged commit 341d579 into JuliaLang:master Aug 24, 2016
@stevengj stevengj deleted the fuse_inline_scalars2 branch August 24, 2016 14:05
stevengj added a commit that referenced this pull request Aug 24, 2016
mfasi pushed a commit to mfasi/julia that referenced this pull request Sep 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broadcast Applying a function over a collection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants