[Target] Creating Target from JSON-like Configuration

[junrushao]

Per RFC, we want to construct complicated target using dicts, e.g.

{
    "kind": "cuda",
    "tag": "nvidia/tx2-cudnn",
    "keys": ["cuda", "gpu"],
    "libs": ["cudnn"],
    "target_host": {
        "kind": "llvm",
        "system_lib": True,
        "mtriple": "aarch64-linux-gnu",
        "mattr": ["+neon"]
    }
}

CC: @jwfromm @tqchen @comaniac

[junrushao]

This PR is ready for review. Please take a look. Thanks!

[comaniac]

I can see some limitations for the current implementation:
- Target string cannot parse Map attributes.
- The target with Map attributes cannot be serialized to a string.
- The target cannot be serialized to a JSON (in string or whatever format).

I know that we are going to deprecate the string serialization feature of target and fully embrace JSON representation in the future, but it'd be fine to have some TODOs or comments mentioning the deprecation in the code. We can also pay less attention to the code with deprecation mark, such as whether to reuse the parsing attribute code in ParseAttrsFromRaw.

In addition, I didn't find an API to generate a JSON string from target. If this will be added in future PRs, please add the full plan to the description so that people could know what to expect.

[junrushao]

Per offline discussion with @comaniac:

• According to the RFC, the new Target design is beyond a raw string. It supports arbitrary nesting, more data types, etc. Therefore, converting target to raw string might be deprecated in the future (but converting from raw string will be preserved for simplicity)
• Target serialization to JSON-like structure (tvm::Map actually) is in the near future after this PR is merged.
• I addressed all the TODOs previously left in this PR

[tqchen]

@comaniac please follow up and https://tvm.apache.org/docs/contribute/code_review.html#approve-and-request-changes-explicitly

[comaniac]

I'm not sure if this part could catch exceptions. For example, what's the expected targets from the following configs?

{"kind": "llvm", "keys": ["cpu", "arm_cpu"], "device": "arm_cpu"}
{"kind": "llvm", "keys": ["gpu"]}

Also, if the config is

{"kind": "llvm", "keys": ["arm_cpu", "cpu"]}

Should "device": "arm_cpu" be added to the target?

To me those cases should be covered in the unit test as well.

[junrushao]

The original design of "-keys" semantic is that:

1. user provided keys comes first
2. add device_name if given
3. add default keys in the target_kind
4. deduplicate the keys, only preserve the first appearance of each key.

Therefore, on your first example, the expected keys of the target created is ["cpu", "arm_cpu"].

[comaniac]

Then are the first two examples valid? While I'm not 100% for sure about the first one, I think the second one is definitely invalid, as its keys will be ["gpu", "cpu"].

Also, what happen if device_name is empty as the third example?

[junrushao]

In response to the second example. We can see that the second example exposes an extreme case that the keys can be semantically unnatural, i.e. typically there are should not be targets on both cpu and gpu. It is typically impossible.

To keep the sanity of the workflow, a check must be conducted somewhere; or we have to assume the users are all "grown adults" who can write correct target (which IMO is still valid assumption). IIUC the problem is that should target creation do such check? IMHO, however, we don't need it to be constant magic strings fixed in C++ side of "target", for the following reasons:

• Imagine an external developer who installs TVM using pip/conda, and he just wanted to write all operators in python using his own specific keys for his specific arm device. Are we harsh enough to ask him to recompile the entire TVM codebase, just to allow this key to exist? It doesn't really let you gain anything, but hurts flexibility.
• It is hardly possible to enumerate all invalid cases. As those two examples you proposed, they are "somewhat invalid" in different ways, reversed key order, semantically incompatible keys, etc. Have they really covered all the cases? IIUC, no, not really. I could imagine thousands of possible combinations that are semantically wrong. For example, "mtriple", "march", "mcpu" are incompatible. It is just impossible to enumerate all of them.

Syntactical vs semantical. We aim at solving syntactical problems in this series of PRs, but as I have mentioned in this post, semantical issues are never possible to be enumerated or solved, and shouldn't be solved under the scope of target. What we really want is a high-level wrap of target creation that makes semantic checking possible, or the "target tag" system that guarantees correct outcome.

Thank you for the very constructive comments! Those comments really made me rethink and understand more deeply into all those problems. Thank you Cody!

[comaniac]

Fair enough. Then I would just have 3 points accordingly:

• We don't aim to have a comprehensive semantic checking in the target parser. Instead, we only insist the basic semantic for users that do not want to spend time on investigating how target should be set. For example, {"kind": "llvm"} for X86, {"kind": "llvm", "device": "arm_cpu"} for ARM.
• Assuming the error handling that the community is working on would be mature shortly, the target with incorrect semantic should be caught somewhere in the build pipeline and a proper error message will be delivered to users.
• As I mentioned before in the online meetup, we really need a document to teach everyone how to set up a proper target to fit the environment. We should open an issue to track the progress of this series of target updating and documentation sould be the last step.

[junrushao]

{"kind": "llvm"} for X86, {"kind": "llvm", "device": "arm_cpu"} for ARM.

This can be done via the helper functions we already have.

the target with incorrect semantic should be caught somewhere in the build pipeline and a proper error message will be delivered to users.

I agree with the spirit, and I did see some mysterious error info when target is incorrect. This is the direction our community should work towards :-)

we really need a document to teach everyone how to set up a proper target to fit the environment

I strongly agree with it. I could see meaning of those target is rarely documented and somehow becomes mysterious. There are cases of misuse in the current codebase too.

[junrushao]

Sorry for the late reply. I was in PTO last two days. Thank you @comaniac for the very constructive comments, and I will reply correspondingly :-)

[junrushao]

@comaniac I just added a few more tests, covering the cases

• Creation failure: "kind" is not provided
• Creation failure: type mismatch
• Duplicate keys
• "Device"/"Keys" is provided

The test for tag will be added after tag is supported.

[comaniac]

LGTM. Thanks :)

[junrushao]

Thank you @comaniac for the valuable feedbacks! Made me think deeper and really learned a lot!

[tqchen]

Thanks @junrushao1994 @comaniac !