[GitHub] [tvm] lazycal commented on pull request #10172: [TIR] Fix Ramp int32~64 mismatch in VectorizeLoop and NarrowDataType passes

[GitBox <gi...@apache.org>]

lazycal commented on pull request #10172:
URL: https://github.com/apache/tvm/pull/10172#issuecomment-1031694343


   > @lazycal Using this impl (based on yours) in https://github.com/lazycal/tvm/blob/ffe6649855c4c247f4bb85c9d48c5ca157850a1d/src/tir/ir/expr.cc#L705 fixes the bug you mentioned and might be more general to overcome other hidden ones if we can assume that `base` and `stride` must be of integers.
   > 
   > ```c++
   > Ramp::Ramp(PrimExpr base, PrimExpr stride, int lanes, Span span) {
   >   ICHECK(base.defined());
   >   ICHECK(stride.defined());
   >   ICHECK(base.dtype().is_scalar());
   >   ICHECK(stride.dtype().is_scalar());
   >   ICHECK_GT(lanes, 1);
   >   ICHECK(base.dtype().is_int());
   >   ICHECK(stride.dtype().is_int());
   >   
   >   if (base.dtype() != stride.dtype()) {
   >     size_t bits = std::max(base.dtype().bits(), stride.dtype().bits());
   >     DataType dtype = base.dtype().with_bits(bits);
   >     if (base.dtype() != dtype) base = cast(dtype, base);
   >     if (stride.dtype() != dtype) stride = cast(dtype, stride);
   >   }
   > 
   >   ObjectPtr<RampNode> node = make_object<RampNode>();
   >   node->dtype = base.dtype().with_lanes(lanes);
   >   node->base = base;
   >   node->stride = stride;
   >   node->lanes = lanes;
   >   node->span = std::move(span);
   >   data_ = std::move(node);
   > }
   > ```
   
   I didn't do what you said because
   - I want to make as little change as possible. Changing a constructor of a fundamental class might be more likely to break things IMO.
   - Also I think the author wrote this class that way must for a reason. Indeed I guess one of them could be to catch such corner cases in pass rewrite algorithms. You are right that implicitly upcasting in the constructor fixes both the bugs, but I am not sure if this is always the desired behavior. For example, there might be some other pass that should have other special handling other than upcasting, and if it forgets to do so, it'd be good to catch that, but implicit upcasting will have it silently ignored or exposed at a much later time.


-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org