AMDMIGraphX icon indicating copy to clipboard operation
AMDMIGraphX copied to clipboard

Horiz fuse after pointwise

Open CharlieL7 opened this issue 11 months ago • 1 comments

Updates find_splits matcher:

  • Skip trying to fuse instructions that have inter-group dependencies (where a split group depends on another such as the case sigmoid(x) + x
  • Allow fusing pointwise instructions that are identical. Currently comparing using string-ified modules.
  • Fixed a bug with dependencies within a group not being detected.

Before this PR in yolox: image After this PR: Screenshot Capture - 2025-03-28 - 17-37-48

CharlieL7 avatar Mar 28 '25 21:03 CharlieL7

Codecov Report

Attention: Patch coverage is 97.61905% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/fuse_pointwise_reduce.cpp 0.00% 1 Missing :warning:
Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #3920   +/-   ##
========================================
  Coverage    92.41%   92.41%           
========================================
  Files          522      522           
  Lines        22532    22555   +23     
========================================
+ Hits         20822    20844   +22     
- Misses        1710     1711    +1     
Files with missing lines Coverage Δ
src/simplify_algebra.cpp 98.26% <100.00%> (+0.04%) :arrow_up:
src/fuse_pointwise_reduce.cpp 0.00% <0.00%> (ø)
:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Mar 31 '25 20:03 codecov[bot]

Unit tests need to be added for fusing the pointwise modules. You will need to add a test suite for the fuse_pointwise_reduce pass and add a test there.

The is_dependent needs to use the distance to avoid traversing the whole graph. The is_dependent function is equivalent to the reaches function from #3870 here. It doesnt avoid traversing the whole graph there, but that will be needed to fix the timeouts in bert, so there is some overlap here.

This really shouldn't have been merged yet as the performance report hasn't even run on this, and there could be timeouts in the compile with this change as well, similar to #3870.

pfultz2 avatar Apr 11 '25 17:04 pfultz2

Unit tests need to be added for fusing the pointwise modules. You will need to add a test suite for the fuse_pointwise_reduce pass and add a test there.

Running fuse_pointwise in one of the simplify_algebra tests. Any difference with moving to fuse_pointwise_reduce?

CharlieL7 avatar Apr 11 '25 18:04 CharlieL7

Running fuse_pointwise in one of the simplify_algebra tests. Any difference with moving to fuse_pointwise_reduce?

Sorry I missed, that should be fine in the simplify_algebra tests.

pfultz2 avatar Apr 11 '25 20:04 pfultz2