quic-sanirudh commented on PR #16795: URL: https://github.com/apache/tvm/pull/16795#issuecomment-2020458701
> In this particular case (deep equality), i think type do matter, so it would be great instead to fix the cases that would depend on the relaxed behavior. > > I know we had some i64/i32 issues, and general rule of thumb now is to try to be explicit as much as possible and that helps to reduce errors Oh okay, thanks for the feedback @tqchen. The cases we started seeing was that some expressions were not getting simplified properly after [`RampNode` lanes were changed to PrimExpr](https://github.com/apache/tvm/commit/a6157a6369c184b6fa5f66654feb685e58726737#diff-046cdcb6494a6719465080bb9156cd4620828af4b18f7018e5b443d6c7c1c1d0L792-R792). I narrowed down the exact simplification that was failing was actually a [rewrite simplify rule here](https://github.com/apache/tvm/blob/main/src/arith/rewrite_simplify.cc#L401). I realized that the simplification was not happening because lanes between broadcast and RampNode in this case had different types, so a couple other solutions I thought would apply here is to either fix the RampNode constructor to stick to some fixed dtype for lanes (something like int32/int16, since `DLDataType` anyways only supports int16 dtype), or to update the simplify rules here to try the same rules with an `PVar<IntImm>` lanes type in case of fixed length vectors. But if dtype does matter, then should we update the `PEqualChecked<IntImm>` to also check for dtypes? -- 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]
