Skip to content

Conversation

@zhiics
Copy link
Member

@zhiics zhiics commented Sep 25, 2018

This PR mainly focuses on supporting multiple devices for tvm.build, it is part of #1688.

The proposal 2 of #1752 is implemented to support multiple devices for compilation and code generation. In the end, only one module will be generated. (In compiler PR, in order to pass target: loweredfunc_list pairs to tvm.build, we need to slightly modify the GraphComplie pass. Multiple targets need to be glued to the graph for compiling, and a device_type field may need to be added toNodeAttr or Op. )

@zhiics zhiics force-pushed the build branch 2 times, most recently from 2677b14 to 8462167 Compare September 26, 2018 01:40
@srkreddy1238
Copy link
Contributor

@zhiics

Ref. RFC #1242

  • It talks about FComputeFallback which indicates the operator to have a fallback to CPU.

Here the device_types is selected based on the result of FComputeFallBack not by explicit arguments to nnvm build. We could continue to have targets passed to nnvm build to control the fall back devices.
Am I missing anything here ?

How about another PR first to demonstrate the fallback behavior of operator and copy insertion before integrating nnvm build to tvm build.

@zhiics
Copy link
Member Author

zhiics commented Sep 26, 2018

@srkreddy1238

device_types is not selected by explicit argument to nnvm build here. It is determined through annotation which will be another PR. I think FComputeFallBack is registered to tell the compiler that the operator should fallback to the host (cpu). This could be done at the annotation pass. I did this first is because I think annotation may need more discussion.

Or I can remove the compiler related stuff now, but only keep the new tvm.build part to reduce confusion of this PR. How do you think?

@srkreddy1238
Copy link
Contributor

Thanks @zhiics

Yes, different PR after with Fallback mechanism followed by integrating it to TVM build makes easy to understand the end to end flow while review.

for devices coupled with target information.

There are two typical uses of this functioin:
1. For backward compatibility
Copy link
Member

Choose a reason for hiding this comment

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

This is a valid case of usage so we don't want to see for backward compatiblity.

Consider put these code examples under Example section with rst real code block

return ir_pass.MakeAPI(stmt, name, arg_list, 0, cfg.restricted_func)


def build(sch,
Copy link
Member

Choose a reason for hiding this comment

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

Let us rename sch to inputs, since now it is more than a scheduld

raise ValueError("Duplicate function name %s" % x.name)
fname_set.add(x.name)

fhost_all = []
Copy link
Member

Choose a reason for hiding this comment

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

Let us move the older per device logic to a separate internal function

@zhiics zhiics force-pushed the build branch 3 times, most recently from 8558960 to e18c0d3 Compare September 28, 2018 15:53
@tqchen tqchen merged commit 47e57be into apache:master Sep 29, 2018
@tqchen
Copy link
Member

tqchen commented Sep 29, 2018

Thanks @srkreddy1238 @zhiics , this is now merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants