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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to