[MacOS] Add MLProgram Gather op for CoreML EP #24387
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
onnxruntime/core/providers/coreml/builders/impl/gather_op_builder.cc
Outdated
Show resolved
Hide resolved
carzh
force-pushed
the
carzh/coreml-gather
branch
from
94a986d to
2591a76
Compare
onnxruntime/core/providers/coreml/builders/impl/gather_op_builder.cc
Outdated
Show resolved
Hide resolved
| // from: int64_data/raw, to: ints (use narrow to convert to int32) | ||
| // CoreML uses int32 |
There was a problem hiding this comment.
is this something we want to do for all int64 inputs?
There was a problem hiding this comment.
Yeah, every CoreML op should have their int64 inputs converted to int32 and their int64 outputs converted to int32. CoreML doesn't support int64 for their ops (though I believe their spec includes a longints field). The path for adding constants already handles conversion from int64 to int32
…der.cc Co-authored-by: Edward Chen <[email protected]>
| try { | ||
| return narrow<int32_t>(v); | ||
| } catch (const std::exception& e) { | ||
| ORT_THROW("Error converting int64 to int32 for tensor ", tensor_name, ": ", e.what(), | ||
| ". Value (", v, ") exceeds int32 range."); | ||
| } |
There was a problem hiding this comment.
it's nice that you're including more detail in the message.
however, generally, we try to avoid using try or catch directly in order to support builds with exceptions disabled. that's why those ORT_THROW and ORT_CATCH macros are there.
in this case, I think we can just call narrow and not bother with adding more info to the exception. if exceptions are enabled, it should be possible to follow a call stack back to this code.
Description
Add MLProgram implementation for Gather
To support this change, I also added handling for converting int64 to int32 in model builder