Aharrypotter commented on code in PR #19649:
URL: https://github.com/apache/tvm/pull/19649#discussion_r3329846334
##########
python/tvm/relax/frontend/tflite/tflite_frontend.py:
##########
@@ -2162,6 +2163,44 @@ def _convert_stablehlo_sort(self, op):
relax.op.sort(data, axis=int(opts.Dimension()),
descending=descending)
)
+ def _convert_stablehlo_custom_call(self, op):
+ """Convert supported annotation-only STABLEHLO_CUSTOM_CALL targets."""
+ from tflite.StablehloCustomCallOptions import
StablehloCustomCallOptions
+
+ input_tensors = self.get_input_tensors(op)
+ output_tensors = self.get_output_tensors(op)
+ opts = self._get_stablehlo_options(op, StablehloCustomCallOptions)
+ call_target_name = self._decode_tflite_string(opts.CallTargetName())
Review Comment:
there is a real robustness gap here, though the fix belongs one level down
rather than at the call site.
The guard as suggested (`if opts is None` inside
`_convert_stablehlo_custom_call`) is unreachable: `_get_stablehlo_options`
never returns `None`. It always constructs and returns an options object:
```python
def _get_stablehlo_options(self, op, options_cls):
op_options = op.BuiltinOptions2()
...
result = options_cls()
result.Init(op_options.Bytes, op_options.Pos)
return result
```
The actual failure happens when `op.BuiltinOptions2()` returns `None` —
`op_options.Bytes` then raises an opaque `AttributeError` *inside the helper*,
before it ever returns. This affects every StableHLO op that goes through
`_get_stablehlo_options` (CONCATENATE, BROADCAST_IN_DIM, REDUCE, SORT, SCATTER,
CUSTOM_CALL, WHILE, ...), not just custom call.
So I've fixed it centrally in `_get_stablehlo_options` instead, which covers
all of those ops with a single clean diagnostic:
```python
op_options = op.BuiltinOptions2()
if op_options is None:
raise tvm.error.OpNotImplemented(
f"{options_cls.__name__} is required but missing from the operator"
)
```
Added `test_stablehlo_options_missing_payload_unsupported`, which builds a
custom-call op that declares the options type but omits the payload and asserts
the clean `OpNotImplemented`.
The other three suggestions (using `_load_model_from_buffer` in the
rejection tests) were good — applied as well.
--
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]
