[TKW] Fix sympy expr lowering and add some more igemm test shapes #184
Conversation
Signed-off-by: Ivan Butygin <[email protected]>
Signed-off-by: Ivan Butygin <[email protected]>
Signed-off-by: Ivan Butygin <[email protected]>
Signed-off-by: Ivan Butygin <[email protected]>
Signed-off-by: Ivan Butygin <[email protected]>
Signed-off-by: Ivan Butygin <[email protected]>
Signed-off-by: Ivan Butygin <[email protected]>
This reverts commit 3efa309. Signed-off-by: Ivan Butygin <[email protected]>
Hardcode84
force-pushed
the
igemm_shapes
branch
from
1df2d88
to
b64a9ee
Compare
Signed-off-by: Ivan Butygin <[email protected]>
Signed-off-by: Ivan Butygin <[email protected]>
Signed-off-by: Ivan Butygin <[email protected]>
Signed-off-by: Ivan Butygin <[email protected]>
Hardcode84
requested review from
raikonenfnu and
harsh-nod
and removed request for
raikonenfnu
Signed-off-by: Ivan Butygin <[email protected]>
Signed-off-by: Ivan Butygin <[email protected]>
|
@harsh-nod @erman-gurses I found a way to separate perf/non-perf tests using custom markers, don't need any external files, see the updated files and |
Signed-off-by: Ivan Butygin <[email protected]>
| if isinstance(arg, arith_d.ConstantOp): | ||
| value = arg.attributes["value"] | ||
| if isinstance(value, IntegerAttr): | ||
| return int(value) |
There was a problem hiding this comment.
Throw error if it is not an int?
There was a problem hiding this comment.
We can actually encounter vector constants now, so this code is intended to fall through to the return None
|
|
||
| stack.append(operation) | ||
| def muli_fold(lhs, rhs): |
There was a problem hiding this comment.
When does this come up? Is it when you are handling with cases like a * (1 / b) where you first do a * 1 and then divide by b? But wouldn't sympy convert that to a/b ?
There was a problem hiding this comment.
But wouldn't sympy convert that to a/b ?
I'm still seeing a bunch of muli %a, 1 without this, so apparently not. They are benign as they will be cleaned up later by canonicalizations, but it also means we will have to update all our codegen lit tests. With this early folding lit codegen IR is identical.
| return value | ||
|
|
||
| def _group_rationals(stack, count): | ||
| rationals = [] |
There was a problem hiding this comment.
Could you add a comment explaining why we need non rationals first and then rationals? I am assuming that is the purpose of this function?
| return arith_d.muli(lhs, rhs) | ||
|
|
||
| # `x + (a/b)` transformed into `(x*b + a) / b` | ||
| def _add(lhs, rhs): |
There was a problem hiding this comment.
I really like the idea of the Rational and late materialization. It was a much better approach than our previous lambda based approach. Thanks!
Signed-off-by: Ivan Butygin <[email protected]>
Signed-off-by: Ivan Butygin <[email protected]>
* Rework how we are lowering `rational` sympy expressions, instead of delayed materialization via lambdas introduce `_Rational` type and propagate `numerator/denominator` values independently. Division will only be materialized on explicit `sympy.floor/ceiling` op. * Rework how igemm test cases are generated and introduce few real shapes. * Use custom pytest markers to separate perf/non-perf tests --------- Signed-off-by: Ivan Butygin <[email protected]>
rationalsympy expressions, instead of delayed materialization via lambdas introduce_Rationaltype and propagatenumerator/denominatorvalues independently. Division will only be materialized on explicitsympy.floor/ceilingop.