Skip to content

[Unity][Fix] Memory planning check value type of 'tir_var_upper_bound' - #16362

Merged
junrushao merged 1 commit into
apache:unityfrom
MasterJH5574:unity-dev/2024-01-07-mem-plan-type-check
Jan 9, 2024
Merged

junrushao merged 1 commit into
apache:unityfrom
MasterJH5574:unity-dev/2024-01-07-mem-plan-type-check

Conversation

@MasterJH5574

Copy link
Copy Markdown
Contributor

This PR adds a sanity check to ensure that all tir_var_upper_bound attrs used by static memory planning has integer as value type. This check helps avoid mistakes of using wrong value types.

The check is needed since func->GetAttr<Map<String, IntImm>> does not apply type check.

Comment thread src/relax/transform/static_plan_block_memory.cc Outdated
@Hzfengsy

Hzfengsy commented Jan 8, 2024

Copy link
Copy Markdown
Member

Thanks for the fix. Beyond this commits, should we also check the value >= 0?

This PR adds a sanity check to ensure that all `tir_var_upper_bound`
attrs used by static memory planning has integer as value type.
This check helps avoid mistakes of using wrong value types.

The check is needed since `func->GetAttr<Map<String, IntImm>>`
does not apply type check.
@MasterJH5574
MasterJH5574 force-pushed the unity-dev/2024-01-07-mem-plan-type-check branch from fb1b236 to 64f66c1 Compare January 8, 2024 17:19
@MasterJH5574

Copy link
Copy Markdown
Contributor Author

Thanks @junrushao and @Hzfengsy for the suggestions! Just updated the PR. Please take another look at your convenience.

@junrushao junrushao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@junrushao
junrushao merged commit a796023 into apache:unity Jan 9, 2024
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.

3 participants