-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
WIP: Optimize exception frames allocation #17760
Conversation
e905033
to
15082a9
Compare
Travis queued here https://travis-ci.org/JuliaLang/julia/builds/149299535.... somehow not showing up again... |
It shows up eventually, but only once the build has started, not when it's pending. I think this is a bug on Travis' side. travis-ci/travis-ci#6342 |
78b3533
to
1cb48fc
Compare
auto add_inst = [&] (BasicBlock::iterator it) { | ||
auto I = &*it; | ||
if (visited.find(I) == visited.end()) { | ||
visited.insert(I); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can fold these two lines as visited.insert(T).second
, iirc
c61a81d
to
7114cea
Compare
I think this looks valid and OK to merge |
* Make llvm-gcroot a ModulePass * Use llvm-gcroot to calculate the minimum number of exception frames necessary.
7114cea
to
3e0c935
Compare
Rebased and fixed LLVM 3.3 compilation. Will merge when CI is done then. |
@@ -4329,18 +4316,6 @@ static std::unique_ptr<Module> emit_function(jl_lambda_info_t *lam, jl_llvm_func | |||
allocate_gc_frame(b0, &ctx); | |||
|
|||
// step 8. allocate space for exception handler contexts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I didn't remove it in the first version to avoid conflicts but forgot to do that before the final rebase....
// the corresponding leaves and collect a list of nested exception frames. | ||
// This assumes the exception frames has simple structure. E.g. | ||
// there's no recursion and different frames do no share the same leave. | ||
std::function<void(hdlr_iter_t)> process_handler = [&] (hdlr_iter_t hdlr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
msvc says c:/projects/julia/src/llvm-gcroot.cpp(221) : error C2039: 'function' : is not a member of 'std'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#include <functional>
http://en.cppreference.com/w/cpp/utility/functional/function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that got further, but now
llvm-gcroot.cpp
c:/projects/julia/src/llvm-gcroot.cpp(362) : error C2280: 'std::unique_ptr<std::vector<llvm::CallInst *,std::allocator<_Kty>>,std::default_delete<_Ty>> &std::unique_ptr<_Ty,std::default_delete<_Ty>>::operator =(const std::unique_ptr<_Ty,std::default_delete<_Ty>> &)' : attempting to reference a deleted function
with
[
_Kty=llvm::CallInst *
, _Ty=std::vector<llvm::CallInst *,std::allocator<llvm::CallInst *>>
]
C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\INCLUDE\memory(1487) : see declaration of 'std::unique_ptr<std::vector<llvm::CallInst *,std::allocator<_Kty>>,std::default_delete<_Ty>>::operator ='
with
[
_Kty=llvm::CallInst *
, _Ty=std::vector<llvm::CallInst *,std::allocator<llvm::CallInst *>>
]
This diagnostic occurred in the compiler generated function '`anonymous-namespace'::HandlerData &`anonymous-namespace'::HandlerData::operator =(const `anonymous-namespace'::HandlerData &)'
make[1]: *** [llvm-gcroot.o] Error 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
diff --git a/src/llvm-gcroot.cpp b/src/llvm-gcroot.cpp
index b722199..51c1a9d 100644
--- a/src/llvm-gcroot.cpp
+++ b/src/llvm-gcroot.cpp
@@ -191,6 +191,11 @@ struct HandlerData {
std::unique_ptr<std::vector<CallInst*>> parent_vec;
CallInst *parent{nullptr};
bool processed{false};
+ HandlerData() = default;
+ HandlerData(HandlerData&&) = default;
+ HandlerData(const HandlerData&) = delete;
+ HandlerData &operator=(HandlerData&&) = default;
+ HandlerData &operator=(const HandlerData&) = delete;
};
void JuliaGCAllocator::lowerHandlers()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
llvm-gcroot.cpp
c:/projects/julia/src/llvm-gcroot.cpp(196) : error C2610: '`anonymous-namespace'::HandlerData::HandlerData(`anonymous-namespace'::HandlerData &&)' : is not a special member function which can be defaulted
c:/projects/julia/src/llvm-gcroot.cpp(198) : error C2610: '`anonymous-namespace'::HandlerData &`anonymous-namespace'::HandlerData::operator =(`anonymous-namespace'::HandlerData &&)' : is not a special member function which can be defaulted
c:/projects/julia/src/llvm-gcroot.cpp(214) : error C2264: '`anonymous-namespace'::HandlerData::operator =' : error in function definition or declaration; function not called
make[1]: *** [llvm-gcroot.o] Error 2
This is an exercise for implementing a more precise liveness analysis and should also be useful for fixing #15369 without suffering from #17288.
The algorithm should be complete. Mark as WIP since this needs more cleanup, testing and possibly fixing compatibility with old LLVM versions. Should also wait after branching.