Skip to content

Conversation

@wrongtest-intellif
Copy link
Contributor

Fix a minor issue that for i in T.serial(0, 4, annotations={"pragma_key":"value"}) result to a runtime.String typed dict value. The flatten pass do not allow attribute value of runtime.String .

@vinx13
Copy link
Member

vinx13 commented Dec 16, 2021

Annotations actually expect String as the key, can we fix the pass instead?

@wrongtest-intellif
Copy link
Contributor Author

wrongtest-intellif commented Dec 17, 2021

@vinx13 Hi, the issue is about string dict value as a clarification. The key type seems no problem. The parser already convert str to StringImm in annotations but provide the old dict (maybe a typo).

For the pass, currently it assumes all dict values should be of PrimExpr in annotations. Did you mean we can allow some "compatible" values to convert to PrimExpr instead of simple downcasts?
https://github.com/apache/tvm/blob/main/src/tir/transforms/flatten_buffer.cc#L99-L106

@vinx13
Copy link
Member

vinx13 commented Dec 17, 2021

StringImm here might be something I forgot to change in #9742. Annotations value is ObjectRef and we would like to use String directly.
In FlattenBuffer, due to compatibility issue with legacy AttrStmt, we should handle String annotation value and create StringImm in that case

@wrongtest-intellif
Copy link
Contributor Author

handle String annotation value and create StringImm

I update it by the suggestion. But another problem is detected. Primfunc with flattened loop attrs seems not supported by script parser. It would lead to "use before def" errors.

def main(A: T.Buffer[(16,), "float32"]) -> None:
        # body
        T.attr(i, "pragma_1", "str_value")
        ...
        for i in T.serial(16):

@junrushao
Copy link
Member

@Hzfengsy is that exactly the same issue we talked about a while ago?

BTW please rebase to our latest HEAD

@wrongtest-intellif wrongtest-intellif force-pushed the fix_parse_strimm_value_in_for_annotation branch from 3e25769 to 8309a4f Compare December 20, 2021 03:05
@Hzfengsy Hzfengsy merged commit f4af81c into apache:main Dec 20, 2021
@Hzfengsy
Copy link
Member

Thanks @wrongtest @vinx13 @junrushao1994

ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
…#9755)

* fix parse strimm value in for annotations

* flatten buffer allow runtime.String attr value

* remove unused import

* rebase and ensure flattened attr order
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
…#9755)

* fix parse strimm value in for annotations

* flatten buffer allow runtime.String attr value

* remove unused import

* rebase and ensure flattened attr order
qsqqsqqsq-intellif pushed a commit to qsqqsqqsq-intellif/tvm that referenced this pull request Apr 29, 2022
…#9755)

* fix parse strimm value in for annotations

* flatten buffer allow runtime.String attr value

* remove unused import

* rebase and ensure flattened attr order
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

Successfully merging this pull request may close these issues.

4 participants