javierdejesusda commented on code in PR #19756:
URL: https://github.com/apache/tvm/pull/19756#discussion_r3408091564
##########
python/tvm/relax/frontend/torch/base_fx_graph_translator.py:
##########
@@ -410,6 +410,28 @@ def _logical_not(self, node: fx.Node) -> relax.Var:
x = self.block_builder.emit(relax.op.astype(x, "bool"))
return self.block_builder.emit(relax.op.logical_not(x))
+ def _logical_or(self, node: fx.Node) -> relax.Var:
+ lhs = self.env[node.args[0]]
+ rhs = self.env[node.args[1]]
+ # torch.logical_or accepts any dtype (treating nonzero as True) and
returns bool, but
+ # relax.op.logical_or requires boolean inputs, so cast non-bool inputs
to bool first.
+ if lhs.struct_info.dtype != "bool":
+ lhs = self.block_builder.emit(relax.op.astype(lhs, "bool"))
+ if rhs.struct_info.dtype != "bool":
+ rhs = self.block_builder.emit(relax.op.astype(rhs, "bool"))
+ return self.block_builder.emit(relax.op.logical_or(lhs, rhs))
+
+ def _logical_xor(self, node: fx.Node) -> relax.Var:
+ lhs = self.env[node.args[0]]
+ rhs = self.env[node.args[1]]
+ # torch.logical_xor accepts any dtype (treating nonzero as True) and
returns bool, but
+ # relax.op.logical_xor requires boolean inputs, so cast non-bool
inputs to bool first.
+ if lhs.struct_info.dtype != "bool":
+ lhs = self.block_builder.emit(relax.op.astype(lhs, "bool"))
+ if rhs.struct_info.dtype != "bool":
+ rhs = self.block_builder.emit(relax.op.astype(rhs, "bool"))
+ return self.block_builder.emit(relax.op.logical_xor(lhs, rhs))
Review Comment:
I kept these as dedicated `_logical_or` / `_logical_xor` methods to stay
consistent with the existing `_logical_and` (#19679) and `_logical_not`
converters, which are also written out individually rather than routed through
a shared helper. Happy to consolidate all of the `_logical_*` converters into
one helper in a follow-up if the maintainers would prefer that as a separate
cleanup.
On the `getattr` guard: these converters only run on `logical_or` /
`logical_xor` nodes, whose operands are always tensors, so `struct_info` is a
`TensorStructInfo` and `.dtype` is always present. This matches how the merged
`_logical_and` accesses `.dtype` directly, so I kept it the same for
consistency.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
