interpreter: add cast mechanism #1264
Conversation
superlopuh
requested review from
compor,
AntonLydike,
georgebisbas,
PapyChacal and
adutilleul
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1264 +/- ##
==========================================
+ Coverage 89.05% 89.14% +0.08%
==========================================
Files 179 182 +3
Lines 23916 24154 +238
Branches 3628 3679 +51
==========================================
+ Hits 21299 21532 +233
- Misses 2040 2042 +2
- Partials 577 580 +3
☔ View full report in Codecov by Sentry. |
| class BuiltinFunctions(InterpreterFunctions): | ||
| @impl(UnrealizedConversionCastOp) | ||
| def run_cast( | ||
| self, | ||
| interpreter: Interpreter, | ||
| op: UnrealizedConversionCastOp, | ||
| args: tuple[Any, ...], | ||
| ): | ||
| return tuple( | ||
| interpreter.cast_value(o.type, r.type, arg) | ||
| for (o, r, arg) in zip(op.operands, op.results, args) | ||
| ) |
There was a problem hiding this comment.
Would be nice to have a test case for this one!
There was a problem hiding this comment.
I have to say, I'm not sure that this is the right approach (but maybe I'm overlooking something).
Is there a reason why we have a special cast mechanism in place, when we could just have the UnrealizedConversionCast interpreter implementation? What do we gain from adding this complexity?
In the Toy pipeline, it's the fake accelerator for pointwise addition and multiplication that uses this mechanism, not the builtin dialects. This is kind of a composition vs inheritance thing, if I have a toy vector extension with some casts, and maybe some IO thing with some casts, how do they both override the implementation of casting in the builtin interpreter functions that lets them both work without knowing about each other? |
|
Hmm, okay fair I think, works for me! |
No description provided.