convert algorithms to SCC #47866
convert algorithms to SCC #47866
Conversation
These places in the code can either be more efficient O(1) or more correct using something more similar to the published SCC algorithm by Tarjan for strongly connected components.
| @@ -1074,13 +1017,15 @@ static int jl_verify_graph_edge(jl_array_t *edges, int idx, htable_t *visited, i | |||
| // Visit all entries in edges, verify if they are valid | |||
| static jl_array_t *jl_verify_graph(jl_array_t *edges, htable_t *visited) | |||
| { | |||
| arraylist_t stack; | |||
| arraylist_new(&stack, 0); | |||
There was a problem hiding this comment.
Is there an arraylist_reserve?
There was a problem hiding this comment.
we have no guess for the amount to reserve, so this is already optimal
| { | ||
| jl_method_instance_t *caller = (jl_method_instance_t*)jl_array_ptr_ref(edges, idx * 2); | ||
| assert(jl_is_method_instance(caller) && jl_is_method(caller->def.method)); | ||
| int found = (char*)ptrhash_get(visited, (void*)caller) - (char*)HT_NOTFOUND; | ||
| if (found == 0) | ||
| return 1; // valid | ||
| return 1; // NOTFOUND == valid |
There was a problem hiding this comment.
Can we use an enum for these?
There was a problem hiding this comment.
not easily, since we also end up doing some arithmetic with them
| @@ -1700,76 +1707,70 @@ static void jl_decorate_module(Module &M) { | |||
| #endif | |||
| } | |||
|
|
|||
| // Implements Tarjan's SCC (strongly connected components) algorithm, simplified to remove the count variable | |||
| static int jl_add_to_ee( | |||
There was a problem hiding this comment.
It would be nice if this could return the SCCs as merged modules instead of committing them to the JIT, since that provides an extension point for any future work operating on the SCCs of a particular compile.
There was a problem hiding this comment.
That is true, it would give cleaner separation of responsibilities to only return the vector of lists of SCCs in order. But also more annoying to preserve that list. Ideally we would not merge them at all, but OrcJIT deleted the API for passing in SCC groups just as we were about ready to use it.
There was a problem hiding this comment.
I guess we can address this in a follow-up, to consider it in isolation.
These places in the code can either be more efficient O(1) or more correct using something more similar to the published SCC algorithm by Tarjan for strongly connected components. (cherry picked from commit b03439c)
These places in the code can either be more efficient O(1) or more correct using something more similar to the published SCC algorithm by Tarjan for strongly connected components.