[Arith] Fix detect linear equation with uint var

[Lucien0]

Hi, the following code encounters an error while executing

c = te.var("c", "uint32")
m = tvm.arith.detect_linear_equation(128 - c, [c])

It seems that when creating coefficients for var, it always uses var's dtype, which doesn't seem very reasonable. Even if var itself is uint, its coefficient is likely to be negative.

[tvm-bot]

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

• cc @junrushao _{See #10317 for details}

_{Generated by tvm-bot}

[quic-sanirudh]

LGTM, thanks.

[tqchen]

Thank you @Lucien0 . we always suggest only using signed integer for index calculations(and provide analysis to detect their signs, or declare as size_var, which is signed but ensures to be non-negative) to avoid cases like underflow. This is because in a lot of cases unsigned integer can have surprises. I know it is not properly documented, so maybe a better approach is to document this rationale in the arith function docs.

Having this explicit choice helps to simplify our implementations and avoid other surprise cases

[Johnson9009]

Thank you @Lucien0 . we always suggest only using signed integer for index calculations(and provide analysis to detect their signs, or declare as size_var, which is signed but ensures to be non-negative) to avoid cases like underflow. This is because in a lot of cases unsigned integer can have surprises. I know it is not properly documented, so maybe a better approach is to document this rationale in the arith function docs.

Having this explicit choice helps to simplify our implementations and avoid other surprise cases

Thanks @tqchen for the comment, let me explain some background information, current we are actively develop our DSL based on TVM script for our ARM China Zhouyi NPU, during these years we try to land TVM DSL(TE, Hybrid script, ir builder) to NPU, the result isn't good, because all of these methods can't act as a role of C/Open CL replacer, even though we can reach better performance for some operators, but still have some operator need to write by the operator library developers. Until now we have time to continue the DSL work for our NPU, and we find TVM script can do whatever C/Open CL language do in theory, so we change our DSL solution to TVM Script based, we think this work have the potential to be a good demo for TIR on NPU (scalar small CPU+ SIMD VPU + MAC arry) like what BYOC does.

I totally agree we should prefer signed integer for programming, but we can't avoid the DSL user use unsigned var like the sample code shown, and more real senario is we have lots of variables and maybe one of them is unsigned and lead to the others that depend on it become unsigned, the current code will give wrong result instead of unoptimized result, so I think if this fix can be accept, because at least it can fix the wrong result? And does this fix have some side effect?

[Johnson9009]

@tqchen @junrushao @Hzfengsy Can help to continue review this PR? Thanks.

[Hzfengsy]

Thanks for the PR and @tqchen's input. It's good to have a robust arith analysis no matter the choice of loop vars. I support this PR.

Meanwhile, I agree with TQ, that it's better to use signed integer for index calculations

[tqchen]

Thanks for the input. Given the change is simple happy to merge it in

[tqchen]

I would like to add up saying that it is strongly preferable to use signed integers, since unlikely we will be able to have robust arith for all dtypes, mainly because the unsigned arith have underflow issues (as well as the extra complexity due to mixed use of signed unsigned dtypes). So while it is fine to land some cases like this one. I would still strongly recommend to use signed int for most cases