Lunderberg commented on PR #14095: URL: https://github.com/apache/tvm/pull/14095#issuecomment-1442114841
I was thinking on it last night, and am leaning the same as @quic-sanirudh . Since all other scheduling changes validate their pre-conditions, hanving a scheduling change that requires the user (or the automatic schedule optimizer) to validate a pre-condition in order to produce correct code would be rather unexpected. > I don't know if there are other schedule primitives that break the correctness assumption, so please correct me if I'm wrong about that. As far as I know, the only current scheduling primitive that may break the correctness of an operator is `sch.tensorize(...)`, and even then that would only occur if the tensor intrinsic's definition doesn't match the implementation. > Could we perhaps add another argument to the layout_transform primitive to force implicit padding and set it to false by default so that by default implicit padding is disabled and we get an error on None for pad_value That's effectively what the current behavior is. If there is a defined `pad_value`, even something like `T.undef()` which will be removed during lowering and has no runtime equivalent, then implicit padding is allowed. If `pad_value` is None, then implicit padding is not allowed. > This effectively allows any index map. What if we were to weaken the check instead of removing it altogether? To maintain correctness, we don't need to find an inverse, only to prove that the transformation is injective. * Updating `IndexMap::NonSurjectiveInverse` to return a `std::optional<std::pair<IndexMap, PrimExpr>>`, with `std::nullopt` returned when the `DetectIterMap` can't recognize the transformation. * Adding a helper method `bool IndexMap::IsProvablyInjective(Array<Range> initial_ranges)`, which returns true if the function maps each input location maps to a unique output location. This could delegate out to either `DetectIterMap`, `arith::Analyzer`, or some combination of the two. * In `tir/schedule/primitive/layout_transformation.cc`, start by validating the `IndexMap` and raising a schedule error if `index_map->IsProvablyInjective()` returns false. That way, we would still maintain correctness, but could have a looser condition. -- 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]
