Skip to content

[Fix][Relax] Lower bool prod as logical all#19557

Open
ConvolutedDog wants to merge 1 commit into
apache:mainfrom
ConvolutedDog:fix-19551
Open

[Fix][Relax] Lower bool prod as logical all#19557
ConvolutedDog wants to merge 1 commit into
apache:mainfrom
ConvolutedDog:fix-19551

Conversation

@ConvolutedDog
Copy link
Copy Markdown
Contributor

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.

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.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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.

Suggested change
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()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since TVM now requires Python 3.10+, this compatibility issue is no longer relevan.

@ConvolutedDog ConvolutedDog marked this pull request as ready for review May 13, 2026 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [Relax][LLVM] torch.prod with dtype=torch.bool lowers to bool R.prod and fails LLVM codegen

1 participant