[Fix][Relax] Lower bool prod as logical all#19557
Conversation
This PR fixed apache#19551. Bool product has logical-AND semantics and cannot be lowered through TIR Mul for LLVM codegen. Route bool prod through all() and add frontend and legalization coverage for bool R.prod.
There was a problem hiding this comment.
Code Review
This pull request implements support for boolean product operations by mapping them to logical AND, which avoids issues with LLVM codegen for boolean multiplication. Changes include updates to the prod operator implementation in op.cc and expanded test coverage for boolean types across several frontend and transformation tests. A review comment identifies a compatibility issue in test_frontend_onnx.py, where the use of the pipe operator for type unions (list | tuple) breaks support for Python versions earlier than 3.10.
|
|
||
| tvm_out = run_in_tvm(model, inputs=inputs, opset=11) | ||
| tvm_selected = tvm_out[0].numpy() if isinstance(tvm_out, (list, tuple)) else tvm_out.numpy() | ||
| tvm_selected = tvm_out[0].numpy() if isinstance(tvm_out, list | tuple) else tvm_out.numpy() |
There was a problem hiding this comment.
The use of the pipe operator (|) for type unions in isinstance is a Python 3.10+ feature (PEP 604). TVM currently maintains compatibility with older Python versions (e.g., 3.8 and 3.9), and this change will cause a TypeError in those environments. Please revert to the compatible tuple syntax: isinstance(tvm_out, (list, tuple)). Additionally, this change appears to be an unrelated drive-by modification.
| tvm_selected = tvm_out[0].numpy() if isinstance(tvm_out, list | tuple) else tvm_out.numpy() | |
| tvm_selected = tvm_out[0].numpy() if isinstance(tvm_out, (list, tuple)) else tvm_out.numpy() |
There was a problem hiding this comment.
Since TVM now requires Python 3.10+, this compatibility issue is no longer relevan.
This PR fixed #19551.
Bool product has logical-AND semantics and cannot be lowered through TIR Mul for LLVM codegen. Route bool prod through all() and add frontend and legalization coverage for bool R.prod.