lazycal commented on PR #10595: URL: https://github.com/apache/tvm/pull/10595#issuecomment-1088201683
It's finally finished. This PR now aims to fix the dtype mismatch issue brought by constructing TIR's `For` statement. The fix is by limiting `For` constructor to only accept `loop_var`, `min` and `extent` of the same type (as pointed out by @tkonolige). This change may help uncover dtype inconsistency issues in other places, but at a cost of stronger "type-checking" (so might slightly make it harder to construct `For`) and disallowing any special use case requiring different dtypes among `loop_var`, `min` and `extent`, such as `float32` as the `extent`. Not sure about the impact of this restriction. If not desired, there might be other solutions to this problem, e.g.: - We may adapt the codegen to accept such for loop of different dtypes (at least currently LLVM codegen doesn't seem to be compatible). - We may also add one more pass before codegen that "legalizes" such For loop. I am not familiar with various codegen pass, and not sure exactly where to insert this additional legalization pass, so I didn't go down those paths, but happy to discusss more. P.S. I added one exception to the For constructor for `int32` in `min` or `extent` when it is `IntImm` but `int64` in the `loop_var`, in which it will automatically promotes the `IntImm` to `int64`. With this it would be more compatible to the old code base (and more friendly to TIR programmer) I think, and it also sounds safe to me. cc: @masahi @tkonolige -- 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]
