Skip to content
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

transformations: (canonicalize) Arith const reassociation #3364

Merged
merged 8 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions tests/filecheck/dialects/arith/canonicalize.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,31 @@ func.func @test_const_const() {
// CHECK-NEXT: %3 = arith.constant 9.542894e-01 : f32
// CHECK-NEXT: "test.op"(%0, %1, %2, %3) : (f32, f32, f32, f32) -> ()
}

func.func @test_const_var_const() {
%0, %1 = "test.op"() : () -> (f32, f32)
%a = arith.constant 2.9979 : f32
%b = arith.constant 3.1415 : f32
%c = arith.constant 4.1415 : f32
%d = arith.constant 5.1415 : f32

%2 = arith.mulf %0, %a : f32
%3 = arith.mulf %2, %b : f32

%4 = arith.mulf %0, %c fastmath<reassoc> : f32
%5 = arith.mulf %4, %d fastmath<fast> : f32

"test.op"(%3, %5) : (f32, f32) -> ()

return

// CHECK-LABEL: @test_const_var_const
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also prefer to see the checks appear above the tests here, but feel free to ignore

// CHECK-NEXT: %0, %1 = "test.op"() : () -> (f32, f32)
// CHECK-NEXT: %a = arith.constant 2.997900e+00 : f32
// CHECK-NEXT: %b = arith.constant 3.141500e+00 : f32
// CHECK-NEXT: %2 = arith.mulf %0, %a : f32
// CHECK-NEXT: %3 = arith.mulf %2, %b : f32
// CHECK-NEXT: %4 = arith.constant 2.129352e+01 : f32
// CHECK-NEXT: %5 = arith.mulf %4, %0 fastmath<fast> : f32
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the resulting flag the or of both operations?
I am out of my depth here, is it what MLIR does?

Copy link
Collaborator Author

@n-io n-io Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, since we're essentially folding two ops into one, I wasn't entirely sure how to set the flags on the new op. The union set of both ops seemed like a cleaner option as compared to picking either one at random. I am not entirely sure myself as to how mlir would resolve such a case.

// CHECK-NEXT: "test.op"(%3, %5) : (f32, f32) -> ()
}
23 changes: 21 additions & 2 deletions xdsl/dialects/arith.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,19 @@ def get_canonicalization_patterns(cls) -> tuple[RewritePattern, ...]:
return (FoldConstConstOp(),)


class FloatingPointLikeBinaryOpHasFastReassociativeCanonicalizationPatternsTrait(
HasCanonicalizationPatternsTrait
):
@classmethod
def get_canonicalization_patterns(cls) -> tuple[RewritePattern, ...]:
from xdsl.transforms.canonicalization_patterns.arith import (
FoldConstConstOp,
FoldConstsByReassociation,
)

return FoldConstsByReassociation(), FoldConstConstOp()


class FloatingPointLikeBinaryOperation(IRDLOperation, abc.ABC):
"""A generic base class for arith's binary operations on floats."""

Expand Down Expand Up @@ -913,7 +926,10 @@ class Addf(FloatingPointLikeBinaryOperation):
name = "arith.addf"

traits = frozenset(
[Pure(), FloatingPointLikeBinaryOpHasCanonicalizationPatternsTrait()]
[
Pure(),
FloatingPointLikeBinaryOpHasFastReassociativeCanonicalizationPatternsTrait(),
]
)


Expand All @@ -931,7 +947,10 @@ class Mulf(FloatingPointLikeBinaryOperation):
name = "arith.mulf"

traits = frozenset(
[Pure(), FloatingPointLikeBinaryOpHasCanonicalizationPatternsTrait()]
[
Pure(),
FloatingPointLikeBinaryOpHasFastReassociativeCanonicalizationPatternsTrait(),
]
)


Expand Down
88 changes: 72 additions & 16 deletions xdsl/transforms/canonicalization_patterns/arith.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,34 @@ def match_and_rewrite(self, op: arith.Addi, rewriter: PatternRewriter) -> None:
rewriter.replace_matched_op([], [op.rhs])


def _fold_const_operation(
op_t: type[arith.FloatingPointLikeBinaryOperation],
lhs: builtin.AnyFloatAttr,
rhs: builtin.AnyFloatAttr,
) -> arith.Constant | None:
match op_t:
case arith.Addf:
val = lhs.value.data + rhs.value.data
case arith.Subf:
val = lhs.value.data - rhs.value.data
case arith.Mulf:
val = lhs.value.data * rhs.value.data
case arith.Divf:
if rhs.value.data == 0.0:
# this mirrors what mlir does
val = float("inf")
else:
val = lhs.value.data / rhs.value.data
case _:
return
return arith.Constant(builtin.FloatAttr(val, lhs.type))


class FoldConstConstOp(RewritePattern):
"""
Folds a floating point binary op whose operands are both `arith.constant`s.
"""

@op_type_rewrite_pattern
def match_and_rewrite(
self, op: arith.FloatingPointLikeBinaryOperation, rewriter: PatternRewriter, /
Expand All @@ -29,20 +56,49 @@ def match_and_rewrite(
and isinstance(op.rhs.owner, arith.Constant)
and isa(l := op.lhs.owner.value, builtin.AnyFloatAttr)
and isa(r := op.rhs.owner.value, builtin.AnyFloatAttr)
and (cnst := _fold_const_operation(type(op), l, r))
):
rewriter.replace_matched_op(cnst)


class FoldConstsByReassociation(RewritePattern):
"""
Rewrites a chain of
`(const1 <op> var) <op> const2`
as
`folded_const <op> val`

The op must be associative and have the `fastmath<reassoc>` flag set.
"""

@op_type_rewrite_pattern
def match_and_rewrite(
self, op: arith.Addf | arith.Mulf, rewriter: PatternRewriter, /
):
if isinstance(op.lhs.owner, arith.Constant):
const1, val = op.lhs.owner, op.rhs
else:
const1, val = op.rhs.owner, op.lhs

if (
not isinstance(const1, arith.Constant)
or len(op.result.uses) != 1
or not isinstance(u := list(op.result.uses)[0].operation, type(op))
or not isinstance(
const2 := u.lhs.owner if u.rhs == op.result else u.rhs.owner,
arith.Constant,
)
or op.fastmath is None
or u.fastmath is None
or arith.FastMathFlag.REASSOC not in op.fastmath.flags
or arith.FastMathFlag.REASSOC not in u.fastmath.flags
n-io marked this conversation as resolved.
Show resolved Hide resolved
or not isa(c1 := const1.value, builtin.AnyFloatAttr)
or not isa(c2 := const2.value, builtin.AnyFloatAttr)
):
match type(op):
case arith.Addf:
val = l.value.data + r.value.data
case arith.Subf:
val = l.value.data - r.value.data
case arith.Mulf:
val = l.value.data * r.value.data
case arith.Divf:
if r.value.data == 0.0:
# this mirrors what mlir does
val = float("inf")
else:
val = l.value.data / r.value.data
case _:
return
rewriter.replace_matched_op(arith.Constant(builtin.FloatAttr(val, l.type)))
return

if cnsts := _fold_const_operation(type(op), c1, c2):
flags = arith.FastMathFlagsAttr(list(op.fastmath.flags | u.fastmath.flags))
rebuild = type(op)(cnsts, val, flags)
rewriter.replace_matched_op([cnsts, rebuild])
rewriter.replace_op(u, [], [rebuild.result])
Loading