Skip to content

Simplify resize extent#2017

Merged
naoyam merged 3 commits intomainfrom
simplify_resize_extent
Apr 2, 2024
Merged

Simplify resize extent#2017
naoyam merged 3 commits intomainfrom
simplify_resize_extent

Conversation

@naoyam
Copy link
Collaborator

@naoyam naoyam commented Apr 2, 2024

I noticed the extent of a slice output IterDomain is not simplified as intended. Currently we do pattern matching of a - b, but we actually get a + (-b) due to SimplifyingIrBuilder. This is a quick fix to make sure the latter pattern is also detected. For example:

Before: i0 + ( ( fmax(0, ( fmin(i0, 1) )) ) + ( -i0 ) )
After: fmax(0, ( fmin(i0, 1) ))


// Simple pad test
TEST_F(ResizeTest, FusionResizePad1) {
TEST_F(ResizeTest, Pad1) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Simplified the test names as no need to have Fusion anymore.

@naoyam naoyam requested a review from jacobhinkle April 2, 2024 02:02
@naoyam
Copy link
Collaborator Author

naoyam commented Apr 2, 2024

!build --diff

if (bop->getBinaryOpType() == BinaryOpType::Sub) {
sub_rhs = bop->rhs();
} else if (bop->getBinaryOpType() == BinaryOpType::Add) {
// Note that SimplifyingIrBuilder may turn (a - b) to (a + (- b))
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be the crux of it, and it's why the previous pattern was not firing. Thanks for figuring this out!

In the future, perhaps we could perform this simplification in SimplifyingIrBuilder itself. To do so, we would need to "flatten" associative/commutative ops like add and mul, then perform cancellation before unflattening. Then anywhere we use SimplifyingIrBuilder::addExpr to construct a + (b + (c + (-a))) we could return b + c. That kind of transform is already done by simplifyExpr but that is only run automatically in certain cases like index expressions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

See #2020 for the SimplifyingIrBuilder approach.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Commented at the PR

@naoyam naoyam merged commit 7a872f9 into main Apr 2, 2024
@naoyam naoyam deleted the simplify_resize_extent branch April 2, 2024 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants