[IR] Default to empty attributes, instead of NULL #16745
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
quic-sanirudh
approved these changes
Mar 20, 2024
Contributor
quic-sanirudh
left a comment
quic-sanirudh
left a comment
There was a problem hiding this comment.
Thanks a lot for this change. I've run into this issue a couple times as well, so this makes a lot of sense.
Prior to this commit, the default `DictAttrs` for an `IRModule`, `tir::PrimFunc`, `relax::Function`, and `relay::Function` was a null value. At each callsite, the absence of a `DictAttrs` needed to be treated as equivalent to an empty `DictAttrs`. In C++, this typically was done using the `foo->GetAttr` helper function, but in Python it needed to be checked explicitly. That is, every callsite needed to check `if func.attrs is not None and attr_name in func.attrs`, rather than only checking `if attr_name in func.attrs`. Since most functions would have at least one attribute to specify the global symbol, these bugs would often surface when working on unrelated changes. This commit changes the default attribute dictionary from `NullValue<DictAttrs>()` to `DictAttrs()`. This avoids having two separate representations of an object without any attributes, and allows the `if attr_name in func.attrs` pattern in the Python API.
Lunderberg
force-pushed
the
ir_attrs_default_to_empty
branch
from
3e71d64 to
edd5448
Compare
Lunderberg
added a commit
to Lunderberg/tvm
that referenced
this pull request
Mar 25, 2024
Introduced in apache#16745, should be the string `"Composite"`, not the bytes `b"Composite"`.
Lunderberg
added a commit
that referenced
this pull request
Mar 26, 2024
Introduced in #16745, should be the string `"Composite"`, not the bytes `b"Composite"`.
thaisacs
pushed a commit
to thaisacs/tvm
that referenced
this pull request
Apr 3, 2024
* [IR] Default to empty attributes, instead of NULL Prior to this commit, the default `DictAttrs` for an `IRModule`, `tir::PrimFunc`, `relax::Function`, and `relay::Function` was a null value. At each callsite, the absence of a `DictAttrs` needed to be treated as equivalent to an empty `DictAttrs`. In C++, this typically was done using the `foo->GetAttr` helper function, but in Python it needed to be checked explicitly. That is, every callsite needed to check `if func.attrs is not None and attr_name in func.attrs`, rather than only checking `if attr_name in func.attrs`. Since most functions would have at least one attribute to specify the global symbol, these bugs would often surface when working on unrelated changes. This commit changes the default attribute dictionary from `NullValue<DictAttrs>()` to `DictAttrs()`. This avoids having two separate representations of an object without any attributes, and allows the `if attr_name in func.attrs` pattern in the Python API. * Remove no-longer-needed checks on attrs being present * Fix up unit tests * More unit test fixes * Undo erroneous find/replace * A few more unit tests * Provide `DictAttrs.get`
thaisacs
pushed a commit
to thaisacs/tvm
that referenced
this pull request
Apr 3, 2024
Introduced in apache#16745, should be the string `"Composite"`, not the bytes `b"Composite"`.
Lunderberg
added a commit
to Lunderberg/tvm
that referenced
this pull request
Apr 3, 2024
A follow-up to apache#16745. For Relax functions produced in TVMScript, when `R.func_attrs` was not present, the default was set to `None` instead of an empty dictionary.
Lunderberg
added a commit
that referenced
this pull request
Apr 5, 2024
A follow-up to #16745. For Relax functions produced in TVMScript, when `R.func_attrs` was not present, the default was set to `None` instead of an empty dictionary.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior to this commit, the default
DictAttrsfor anIRModule,tir::PrimFunc,relax::Function, andrelay::Functionwas a null value. At each callsite, the absence of aDictAttrsneeded to be treated as equivalent to an emptyDictAttrs. In C++, this typically was done using thefoo->GetAttrhelper function, but in Python it needed to be checked explicitly. That is, every callsite needed to checkif func.attrs is not None and attr_name in func.attrs, rather than only checkingif attr_name in func.attrs.Since most functions would have at least one attribute to specify the global symbol, these bugs would often surface when working on unrelated changes.
This commit changes the default attribute dictionary from
NullValue<DictAttrs>()toDictAttrs(). This avoids having two separate representations of an object without any attributes, and allows theif attr_name in func.attrspattern in the Python API.