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

Jit bind #2726

Merged
merged 26 commits into from Sep 6, 2018
Merged

Jit bind #2726

merged 26 commits into from Sep 6, 2018

Conversation

Hardcode84
Copy link
Contributor

@Hardcode84 Hardcode84 commented Jun 6, 2018

bind-like functionality which allow to generate efficient specialized versions of functions (much like easy::jit).

Add ldc.dynamic_compile.bind function with interface similar to C++ std::bind. Function returns BindPtr object (which is reference counted internally) with opCall method.

Actual body is generated during compileDynamicCode and after that BindPtr can be called.
Function passed to bind must be marked @dynamicCompile, otherwise BindPtr.isCallable() will return false and 'opCall' asserts.

If function bind as parameter marked as @dynamicCompile it will be efficiently optimized (see dynamiccompile/bind_func_opt.d test).

Also bind functions can be chained together (see dynamiccompile/bind_nested_opt.d test).

@Hardcode84 Hardcode84 changed the title WIP: Jit bind Jit bind Jun 9, 2018
@Hardcode84
Copy link
Contributor Author

Ready, I think.

@JohanEngelen
Copy link
Member

Great that you submitted some parts as separate PRs.
Quickly glancing over it, I missed documentation of the new user function bind.

@@ -83,10 +83,306 @@ void compileDynamicCode(in CompilerSettings settings = CompilerSettings.init)
rtCompileProcessImpl(context, context.sizeof);
}

auto bind(F, Args...)(F func, Args args)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add docs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

auto bind(F, Args...)(F func, Args args)
{
import std.format;
static assert(isFunctionPointer!F);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice to add a message to the user when this fails

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

alias FuncParams = Parameters!(F);
alias Ret = ReturnType!F;
F func = null;
static assert(func.offsetof == 0, "func must be fist");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in error msg

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@Hardcode84
Copy link
Contributor Author

just rebase (and something bad happened with commit dates so github display nonsense like 8 days ago)

@Hardcode84
Copy link
Contributor Author

rebased, added docs

return BindPtrType.make!Index(func, mapBindParams!(F, 0)(args).expand);
}

immutable placeholder = _placeholder();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This member should also be documented briefly. Probably just a reference to bind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@Hardcode84
Copy link
Contributor Author

Updated: delegate as first parameter, @dynamicCompileEmit, some docs

@Hardcode84
Copy link
Contributor Author

Rebased.

TODO: probably need to write some design overview

@Hardcode84
Copy link
Contributor Author

ready

@@ -0,0 +1,27 @@
#ifndef BIND_H
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing header

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

//
//===----------------------------------------------------------------------===//
//
// ParamSlice declaration.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

;-) nitpick: this comment is useless, I can read the same a few lines down in C++. What would help is to explain what ParamSlice is used for.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

@@ -0,0 +1,25 @@
# Dynamic compile bind
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that you added some design documentation

@@ -405,7 +405,7 @@ if(LDC_DYNAMIC_COMPILE STREQUAL "AUTO")
if (NOT (LDC_LLVM_VER LESS 500))
set(LDC_DYNAMIC_COMPILE True)
add_definitions(-DLDC_DYNAMIC_COMPILE)
add_definitions(-DLDC_DYNAMIC_COMPILE_API_VERSION=1)
add_definitions(-DLDC_DYNAMIC_COMPILE_API_VERSION=2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure to add an item about this to the release notes.

@JohanEngelen
Copy link
Member

I've made some grammar fixes to the documentation.
Overall, I think it's fine to include, I didn't do a review of the actual tech content of this PR. I trust you know what you are doing :)

@JohanEngelen
Copy link
Member

See some inline comments.

@Hardcode84 Hardcode84 merged commit 3fa6b26 into ldc-developers:master Sep 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants