gemini-code-assist[bot] commented on code in PR #19890:
URL: https://github.com/apache/tvm/pull/19890#discussion_r3476220758
##########
src/relax/op/tensor/qdq.cc:
##########
@@ -69,9 +69,9 @@ Type InferTypeQuantize(const Call& call, const BlockBuilder&
ctx) {
TensorType input_ty = GetInputTensorType(call, ctx)[0];
TensorType scale_ty = GetInputTensorType(call, ctx)[1];
TensorType zp_ty = GetInputTensorType(call, ctx)[2];
- PrimType input_dtype = input_ty->dtype;
- PrimType scale_dtype = scale_ty->dtype;
- PrimType zp_dtype = zp_ty->dtype;
+ PrimType input_dtype = input_ty->dtype.value();
+ PrimType scale_dtype = scale_ty->dtype.value();
+ PrimType zp_dtype = zp_ty->dtype.value();
Review Comment:

If any of the input tensors (`input_ty`, `scale_ty`, or `zp_ty`) has an
unknown dtype, calling `.value()` directly on their `dtype` fields will crash.
Since type inference can be run on expressions with unknown dtypes, we should
check `IsUnknownDtype()` and return a `TensorType` with an unknown dtype
instead of crashing.
```suggestion
if (input_ty->IsUnknownDtype() || scale_ty->IsUnknownDtype() ||
zp_ty->IsUnknownDtype()) {
if (input_ty->shape.defined()) {
return TensorType(input_ty->shape.value(), std::nullopt,
input_ty->vdevice);
} else {
return TensorType(std::nullopt, input_ty->ndim, input_ty->vdevice);
}
}
PrimType input_dtype = input_ty->dtype.value();
PrimType scale_dtype = scale_ty->dtype.value();
PrimType zp_dtype = zp_ty->dtype.value();
```
##########
src/relax/op/tensor/qdq.cc:
##########
@@ -171,9 +171,9 @@ Type InferTypeDequantize(const Call& call, const
BlockBuilder& ctx) {
TensorType input_ty = GetInputTensorType(call, ctx)[0];
TensorType scale_ty = GetInputTensorType(call, ctx)[1];
TensorType zp_ty = GetInputTensorType(call, ctx)[2];
- PrimType input_dtype = input_ty->dtype;
- PrimType scale_dtype = scale_ty->dtype;
- PrimType zp_dtype = zp_ty->dtype;
+ PrimType input_dtype = input_ty->dtype.value();
+ PrimType scale_dtype = scale_ty->dtype.value();
+ PrimType zp_dtype = zp_ty->dtype.value();
Review Comment:

If any of the input tensors (`input_ty`, `scale_ty`, or `zp_ty`) has an
unknown dtype, calling `.value()` directly on their `dtype` fields will crash.
Since type inference can be run on expressions with unknown dtypes, we should
check `IsUnknownDtype()` and return a `TensorType` with an unknown dtype
instead of crashing.
```c
if (input_ty->IsUnknownDtype() || scale_ty->IsUnknownDtype() ||
zp_ty->IsUnknownDtype()) {
if (input_ty->shape.defined()) {
return TensorType(input_ty->shape.value(), std::nullopt,
input_ty->vdevice);
} else {
return TensorType(std::nullopt, input_ty->ndim, input_ty->vdevice);
}
}
PrimType input_dtype = input_ty->dtype.value();
PrimType scale_dtype = scale_ty->dtype.value();
PrimType zp_dtype = zp_ty->dtype.value();
```
##########
src/relax/op/op_common.h:
##########
@@ -198,16 +198,15 @@ std::tuple<ArgTypes...> GetArgType(const Call& call,
const BlockBuilder& ctx) {
template <bool require_float_dtype, typename FType>
inline Type InferTypeUnary(const Call& call, const BlockBuilder& ctx, FType
f_compute_out_dtype) {
TensorType input_ty = GetUnaryInputTensorType(call, ctx);
- PrimType input_dtype = input_ty->dtype;
if (require_float_dtype && !input_ty->IsUnknownDtype() &&
- !input_dtype.MatchesCode(DLDataTypeCode::kDLFloat,
DLDataTypeCode::kDLBfloat)) {
+ !input_ty->dtype.value().MatchesCode(DLDataTypeCode::kDLFloat,
DLDataTypeCode::kDLBfloat)) {
TVM_FFI_VISIT_THROW(TypeError, call)
<< call->op
<< " requires the input tensor to have float dtype. However, the given
input dtype is "
<< input_ty->dtype;
Review Comment:

Since we have already verified that `!input_ty->IsUnknownDtype()`, we should
print `input_ty->dtype.value()` instead of the optional wrapper
`input_ty->dtype` to ensure the error message displays a clean concrete dtype
(e.g., `float32`).
```suggestion
<< input_ty->dtype.value();
```
##########
src/relax/transform/to_mixed_precision.cc:
##########
@@ -315,7 +315,7 @@ class ToMixedPrecisionRewriter : public ExprMutator {
if (NTypeEqual()(to[0], NTypeFrom(expr))) return expr;
// We only rewrite the expr if the dtype is fp16 or fp32, dtypes such as
int32, float64 is not
// supported to be rewritten
- DLDataType tensor_dtype = tensor->dtype->dtype;
+ DLDataType tensor_dtype = tensor->dtype.value()->dtype;
Review Comment:

If `tensor->IsUnknownDtype()` is true, calling `tensor->dtype.value()` will
crash. We should check if the tensor has an unknown dtype and return the
original expression early.
```suggestion
if (tensor->IsUnknownDtype()) return expr;
DLDataType tensor_dtype = tensor->dtype.value()->dtype;
```
##########
src/relax/utils.cc:
##########
@@ -183,7 +183,7 @@ bool IsBoolType(const Type& ty, bool permit_unknown_rank,
bool permit_unknown_dt
int ndim;
if (const auto* tensor = ty.as<TensorTypeNode>()) {
- dtype = tensor->dtype->dtype;
+ dtype = tensor->dtype.value()->dtype;
ndim = tensor->ndim;
Review Comment:

When `permit_unknown_dtype` is true and `tensor->IsUnknownDtype()` is true,
calling `tensor->dtype.value()` will throw an exception or crash due to
accessing an empty optional. We should safely handle the unknown dtype case
when `permit_unknown_dtype` is enabled.
```c
if (const auto* tensor = ty.as<TensorTypeNode>()) {
if (tensor->IsUnknownDtype()) {
if (!permit_unknown_dtype) {
return false;
}
dtype = DLDataType{kDLOpaqueHandle, 0, 0};
} else {
dtype = tensor->dtype.value()->dtype;
}
ndim = tensor->ndim;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]