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:
   ![high](https://www.gstatic.com/codereviewagent/high-priority.svg)
   
   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:
   ![high](https://www.gstatic.com/codereviewagent/high-priority.svg)
   
   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:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   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:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   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:
   ![critical](https://www.gstatic.com/codereviewagent/critical.svg)
   
   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]

Reply via email to