One: Compiler FE: Revise the way dependency tree is generated

Created on 6 Apr 2021  路  7Comments  路  Source: Samsung/ONE

What

Let's talk about the way dependency tree is generated

Why

We are currently resolving dependencies between modules by calling add_subdirectory in a topological order.
https://github.com/Samsung/ONE/blob/da089b410f1a959a46a99c15ad0f63444cdb9f87/compiler/CMakeLists.txt#L29-L31

Each module that needs to resolve dependencies has requires.cmake file that lists its dependencies. These are parsed and then given to the input of above tsort command. We had been used this method successfully in so long. but an exception has occurred where circle-part-eval module couldn't be built even though it has the requires.cmake.

Actually, add_subdirectory won't make a dependency tree. It is add_dependencies or target_link_libraries who is responsible for that functionality. So, we can make things better by changing the way dependency tree is generated. Also, it would be good to use existing requires.cmake with macro to make less changes.

Until this issue is resolved, we can just add add_dependencies command individually as a workaround at the point where the dependency issue has occurred like this PR.

@parjong Please give me some opinion about this issue:)

typdiscussion

All 7 comments

Maybe close this issue?

@seanshpark I mistakenly mentioned #6052. This is not related that issue at all.

6447 resolved the build break with add_dependencies(circle_part_eval_prepare common_artifacts_deps) - work around, which means that requires.cmake isn't working as we think it should be.

OK, let's hear from @parjong next week :)

add_dependencies work around ... requires.cmake isn't working as we think it should be

I don't think this is a work around... and I don't know what you expected from requires.cmake but I think requires.cmake is working as it should be...

@seanshpark

$ cat compiler/circle-part-value-test/requires.cmake 
require("common-artifacts")
require("circle-partitioner")
require("circle-part-driver")

But, even though circle-part-value-test set require("common-artifacts"), add_dependencies(circle_part_eval_prepare common_artifacts_deps) was needed, which command wouldn't have been needed if requires.cmake had worked well.

which command wouldn't have needed if requires.cmake had worked well.

um... have you seen inside require() implementation?
and could you explain a bit more what you expect from it ?
and how to make it possible with cmake script ?

@seanshpark You're right. I totally misunderstood about this:(
In top CMakeLists.txt, add_subdirectory is called in topological order, which reduces duplicate run of add_subdirectory. Without this, we have to append add_subdirectory to every single sub-module's CMakeLists.txt to let cmake know about dependent target that would happen to not being configured at times.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

lucenticus picture lucenticus  路  3Comments

seanshpark picture seanshpark  路  3Comments

jinevening picture jinevening  路  3Comments

dr-venkman picture dr-venkman  路  4Comments

seanshpark picture seanshpark  路  3Comments