Skip to content
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

Upstream Sync3 #57

Closed
tqchen opened this issue Apr 18, 2020 · 4 comments
Closed

Upstream Sync3 #57

tqchen opened this issue Apr 18, 2020 · 4 comments

Comments

@tqchen
Copy link
Contributor

tqchen commented Apr 18, 2020

Might be a good item after apache/tvm#5372

This PR brings most of the TIR close to what we have in this repo. It might be a good chance to sync after that PR lands. Some noticable highlights include:

  • We now use IRModule(of PrimFuncs) primarily for transformations
  • Introduced BufferRealize(which might allow us to reuse StorageFlatten for TensorIR lowering)
  • BufferLoad/Store are now first class citizen, and Provide/Realize/HalideCall will no longer be used in TIR passses.

Some related changes in this repo:

  • Move most items in tvm/tir/ir.h to tvm/tir/expr.h tvm/tir/stmt.h
  • Perhaps we can reuse StorageFlatten by have TensorIRLower to just annotate BufferRealize
@tqchen
Copy link
Contributor Author

tqchen commented Apr 18, 2020

cc @Hzfengsy @spectrometerHBH

@Hzfengsy
Copy link
Member

I'm confused about why PrimFunc does not have the attribute name?

It does bring trouble during hybrid parsing. Before rebasing, we can parse a single function or a complete module from hybrid. However, if PrimFunc has no attr name, we cannot auto construct GlobalVars through the name while parsing a single function.

I tried to use global_symbol in DictAttrs instead. Unfortunately, it brings another problem. All attrs will be checked during structural_equal but the name is not supposed to be compared.

I'd like to bring the attribute name back. Or we can only allow users to write a complete module in hybrid which may bring extra difficulties in use.

@tqchen
Copy link
Contributor Author

tqchen commented Apr 26, 2020

The reason why name is removed is because the term name could mean different things(hint for the function name) or the intended symbol name of the final generated code.

The former one belongs to the name_hint field (in the GlobalVar). The later is the global_symbol attribute.

In our particular case of hybrid parsing, would it be sufficient to return a HybridFunc(which contains the name field) when we run the @tvm.tir.hybrid, and convert to PrimFunc(without global_symbol unless explicitly specified).

When we construct IRModule, we can use the stored name as GlobalVar.

would love to hear more discussions about the API choice.

@Hzfengsy
Copy link
Member

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants