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

Several type mismatch fixes and checks #12041

Merged
merged 8 commits into from
Jul 11, 2022
Merged

Several type mismatch fixes and checks #12041

merged 8 commits into from
Jul 11, 2022

Conversation

kparzysz-quic
Copy link
Contributor

This is a follow-up to #12013. It fixes several places where type mismatches are introduced, also adds a check to IRSubstitute to make sure that the type of the replacement PrimExpr is the same as the type of the Var being replaced.

Krzysztof Parzyszek and others added 5 commits July 8, 2022 07:00
The corresponding dimensions in the input/output tensors in a broadcast
operations may have the same value, but different types (e.g. int32 vs
int64).
When the broadcast helper tries to unify the dimensions it also needs
to compute the common type to hold the dimension.
Only the `min` member was type-casted, which could lead to ranges with
different types for `min` and `extent`.
Move the casts to the argument of Simplify, so that they can be eliminated
if they aren't needed.
In some cases the domain ranges had the `min` and the `extent` values
be of different types (e.g. [(int64)0, 32)). This is an error, and it
can lead to compilation failures later on. Add a check for equal types
here to catch this early.
Also, only add the cast operation when the desired type differs from
the current one to keep the expressions simpler.
Add a check to IRSubstitute to detect when the type of a variable and
the type of the expression to replace it with have different types.
@kparzysz-quic
Copy link
Contributor Author

cc @ganler

Krzysztof Parzyszek added 3 commits July 8, 2022 13:07
When the script parser deals with lambdas, it creates Var objects for each
parameter. Their actual types are not known at the time, and the properly
typed variables are subtituted in the body later. Since the default dtype
of a Var is "int32", this could lead to a type mismatch in Substitute.
To deal with this scenario, use "void" for newly created Vars in the
parser, and add an exception to Substitute to allow replacing void Vars
with expressions of any type.
@masahi masahi merged commit cf15375 into apache:main Jul 11, 2022
@kparzysz-quic kparzysz-quic deleted the subst-type-check branch July 11, 2022 13:26
masahi pushed a commit to masahi/tvm that referenced this pull request Jul 15, 2022
* Compute common type for shape elements in BroadcastHelper

The corresponding dimensions in the input/output tensors in a broadcast
operations may have the same value, but different types (e.g. int32 vs
int64).
When the broadcast helper tries to unify the dimensions it also needs
to compute the common type to hold the dimension.

* Cast and simplify both members of `Range`

Only the `min` member was type-casted, which could lead to ranges with
different types for `min` and `extent`.
Move the casts to the argument of Simplify, so that they can be eliminated
if they aren't needed.

* Type-check iv domain ranges, use cast only if needed in MakeLoopNest

In some cases the domain ranges had the `min` and the `extent` values
be of different types (e.g. [(int64)0, 32)). This is an error, and it
can lead to compilation failures later on. Add a check for equal types
here to catch this early.
Also, only add the cast operation when the desired type differs from
the current one to keep the expressions simpler.

* Check that variable and substituted expression have same types

Add a check to IRSubstitute to detect when the type of a variable and
the type of the expression to replace it with have different types.

* Add testcase

* [TVMScript] Use void for lambda parameters, allow mismatch in Substitute

When the script parser deals with lambdas, it creates Var objects for each
parameter. Their actual types are not known at the time, and the properly
typed variables are subtituted in the body later. Since the default dtype
of a Var is "int32", this could lead to a type mismatch in Substitute.
To deal with this scenario, use "void" for newly created Vars in the
parser, and add an exception to Substitute to allow replacing void Vars
with expressions of any type.

* Fix type error in test_reduce_combiner_simplify

* Restart CI

Co-authored-by: Jiawei Liu <[email protected]>
junrushao pushed a commit to yelite/tvm that referenced this pull request Jul 27, 2022
* Compute common type for shape elements in BroadcastHelper

The corresponding dimensions in the input/output tensors in a broadcast
operations may have the same value, but different types (e.g. int32 vs
int64).
When the broadcast helper tries to unify the dimensions it also needs
to compute the common type to hold the dimension.

* Cast and simplify both members of `Range`

Only the `min` member was type-casted, which could lead to ranges with
different types for `min` and `extent`.
Move the casts to the argument of Simplify, so that they can be eliminated
if they aren't needed.

* Type-check iv domain ranges, use cast only if needed in MakeLoopNest

In some cases the domain ranges had the `min` and the `extent` values
be of different types (e.g. [(int64)0, 32)). This is an error, and it
can lead to compilation failures later on. Add a check for equal types
here to catch this early.
Also, only add the cast operation when the desired type differs from
the current one to keep the expressions simpler.

* Check that variable and substituted expression have same types

Add a check to IRSubstitute to detect when the type of a variable and
the type of the expression to replace it with have different types.

* Add testcase

* [TVMScript] Use void for lambda parameters, allow mismatch in Substitute

When the script parser deals with lambdas, it creates Var objects for each
parameter. Their actual types are not known at the time, and the properly
typed variables are subtituted in the body later. Since the default dtype
of a Var is "int32", this could lead to a type mismatch in Substitute.
To deal with this scenario, use "void" for newly created Vars in the
parser, and add an exception to Substitute to allow replacing void Vars
with expressions of any type.

* Fix type error in test_reduce_combiner_simplify

* Restart CI

Co-authored-by: Jiawei Liu <[email protected]>
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
* Compute common type for shape elements in BroadcastHelper

The corresponding dimensions in the input/output tensors in a broadcast
operations may have the same value, but different types (e.g. int32 vs
int64).
When the broadcast helper tries to unify the dimensions it also needs
to compute the common type to hold the dimension.

* Cast and simplify both members of `Range`

Only the `min` member was type-casted, which could lead to ranges with
different types for `min` and `extent`.
Move the casts to the argument of Simplify, so that they can be eliminated
if they aren't needed.

* Type-check iv domain ranges, use cast only if needed in MakeLoopNest

In some cases the domain ranges had the `min` and the `extent` values
be of different types (e.g. [(int64)0, 32)). This is an error, and it
can lead to compilation failures later on. Add a check for equal types
here to catch this early.
Also, only add the cast operation when the desired type differs from
the current one to keep the expressions simpler.

* Check that variable and substituted expression have same types

Add a check to IRSubstitute to detect when the type of a variable and
the type of the expression to replace it with have different types.

* Add testcase

* [TVMScript] Use void for lambda parameters, allow mismatch in Substitute

When the script parser deals with lambdas, it creates Var objects for each
parameter. Their actual types are not known at the time, and the properly
typed variables are subtituted in the body later. Since the default dtype
of a Var is "int32", this could lead to a type mismatch in Substitute.
To deal with this scenario, use "void" for newly created Vars in the
parser, and add an exception to Substitute to allow replacing void Vars
with expressions of any type.

* Fix type error in test_reduce_combiner_simplify

* Restart CI

Co-authored-by: Jiawei Liu <[email protected]>
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
* Compute common type for shape elements in BroadcastHelper

The corresponding dimensions in the input/output tensors in a broadcast
operations may have the same value, but different types (e.g. int32 vs
int64).
When the broadcast helper tries to unify the dimensions it also needs
to compute the common type to hold the dimension.

* Cast and simplify both members of `Range`

Only the `min` member was type-casted, which could lead to ranges with
different types for `min` and `extent`.
Move the casts to the argument of Simplify, so that they can be eliminated
if they aren't needed.

* Type-check iv domain ranges, use cast only if needed in MakeLoopNest

In some cases the domain ranges had the `min` and the `extent` values
be of different types (e.g. [(int64)0, 32)). This is an error, and it
can lead to compilation failures later on. Add a check for equal types
here to catch this early.
Also, only add the cast operation when the desired type differs from
the current one to keep the expressions simpler.

* Check that variable and substituted expression have same types

Add a check to IRSubstitute to detect when the type of a variable and
the type of the expression to replace it with have different types.

* Add testcase

* [TVMScript] Use void for lambda parameters, allow mismatch in Substitute

When the script parser deals with lambdas, it creates Var objects for each
parameter. Their actual types are not known at the time, and the properly
typed variables are subtituted in the body later. Since the default dtype
of a Var is "int32", this could lead to a type mismatch in Substitute.
To deal with this scenario, use "void" for newly created Vars in the
parser, and add an exception to Substitute to allow replacing void Vars
with expressions of any type.

* Fix type error in test_reduce_combiner_simplify

* Restart CI

Co-authored-by: Jiawei Liu <[email protected]>
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