-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dialects: (stablehlo) add elementwise add, sub, mul #3030
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3030 +/- ##
=======================================
Coverage 89.85% 89.85%
=======================================
Files 413 413
Lines 51852 51871 +19
Branches 8006 8010 +4
=======================================
+ Hits 46592 46610 +18
- Misses 3970 3972 +2
+ Partials 1290 1289 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these operations not have assemblyFormat/custom parsing?
@@ -1,8 +0,0 @@ | |||
// RUN: xdsl-opt %s --parsing-diagnostics --verify-diagnostics --split-input-file | filecheck %s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good it this could have been git mv
ed but not necessary
class ElementwiseBinaryOperation(IRDLOperation, abc.ABC): | ||
# TODO: Remove this constraint for complex types. | ||
T = Annotated[AnyTensorType, ConstraintVar("T")] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these have some verification that the inputs are "numeric"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually we'll make these constrained to the values that the original stablehlo dialect supports, I don't think it's critical right now.
nope |
Also reorganizes the test.
CC @efferifick