Skip to content

[TorchToTosa] Lower bool bitwise ops to logical ops#4490

Open
catcor01 wants to merge 8 commits intollvm:mainfrom
catcor01:bitwise_to_logical
Open

[TorchToTosa] Lower bool bitwise ops to logical ops#4490
catcor01 wants to merge 8 commits intollvm:mainfrom
catcor01:bitwise_to_logical

Conversation

@catcor01
Copy link
Contributor

@catcor01 catcor01 commented Mar 6, 2026

TOSA bitwise_* ops do not accept i1 tensors. When aten.bitwise_* is applied to bool tensors, lower to tosa.logical_and/or/xor instead. Add a conversion test for bool bitwise_and.

Change-Id: I98e0ae72f69c926abe97c036489bf2e39ba6f88f

TOSA bitwise_* ops do not accept i1 tensors. When aten.bitwise_* is
applied to bool tensors, lower to tosa.logical_and/or/xor instead.
Add a conversion test for bool bitwise_and.

Signed-off-by: Cathal Corbett <cathal.corbett@arm.com>
Change-Id: I98e0ae72f69c926abe97c036489bf2e39ba6f88f
@catcor01
Copy link
Contributor Author

catcor01 commented Mar 9, 2026

@sahas3 @Lallapallooza

Copy link
Member

@sahas3 sahas3 left a comment

Choose a reason for hiding this comment

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

LG with a few suggestions.

Comment on lines +755 to +758
auto lhsBool =
tosa::tosaCastTensorToType(rewriter, lhs, resultTy).value();
auto rhsBool =
tosa::tosaCastTensorToType(rewriter, rhsTensor, resultTy).value();
Copy link
Member

Choose a reason for hiding this comment

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

Are these casts necessary? Won't the operand types already be same as the result type for the ops under consideration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I traced the lowering in ConvertAtenCompareOp: resultTy comes from the type converter, but lhs/rhsTensor are still the original operands at that point, before any promotion. So I don't believe we can prove operands are already i1 when resultTy is i1. The casts are therefore defensive to guarantee i1 for Logical*Op.

Copy link
Member

Choose a reason for hiding this comment

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

@catcor01 Can you please clarify this question? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sahas3 apologies, I did not realize my comment has been pending since yesterday.

Copy link
Member

Choose a reason for hiding this comment

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

No worries :)

Change-Id: If46ca296d3d4fbd27ed5f39ed88a17825a23aa4f
Signed-off-by: Cathal Corbett <cathal.corbett@arm.com>
Change-Id: I8770a509a395145fae78cbcc93320279a4c25de9
Signed-off-by: Cathal Corbett <cathal.corbett@arm.com>
@catcor01 catcor01 force-pushed the bitwise_to_logical branch from 983e5b0 to 8911a85 Compare March 10, 2026 13:13
@catcor01 catcor01 requested a review from sahas3 March 12, 2026 07:59
Change-Id: I0c4e3b06d550efa0995fe5e4d5170a6852c2c948
Signed-off-by: Cathal Corbett <cathal.corbett@arm.com>
Copy link
Member

@Lallapallooza Lallapallooza left a comment

Choose a reason for hiding this comment

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

LGTM: few nits

Change-Id: Ib55aaccd05bb09e8815dd2c68eb235c37504f3f4
Signed-off-by: Cathal Corbett <cathal.corbett@arm.com>
Copy link
Member

@sahas3 sahas3 left a comment

Choose a reason for hiding this comment

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

LGTM.

Comment on lines +755 to +758
auto lhsBool =
tosa::tosaCastTensorToType(rewriter, lhs, resultTy).value();
auto rhsBool =
tosa::tosaCastTensorToType(rewriter, rhsTensor, resultTy).value();
Copy link
Member

Choose a reason for hiding this comment

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

No worries :)

Comment on lines 772 to 775
Copy link
Member

Choose a reason for hiding this comment

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

These casts can be done before the isBitwiseOp check instead of 753-756.

Change-Id: I79424d7fa61f0788a5d9ef055312971ca5d98fbd
Signed-off-by: Cathal Corbett <cathal.corbett@arm.com>
Change-Id: Ided255c7522625eeda323f9ffd4febe8478873d5
Signed-off-by: Cathal Corbett <cathal.corbett@arm.com>
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.

3 participants