Skip to content

Enable float16 MatMul+Add -> GEMM fusion for performance boost - #1506

Merged
snnn merged 1 commit into
microsoft:masterfrom
fdwr:user/dwayner/MatMulAddToGemmFusion
Jul 29, 2019
Merged

Enable float16 MatMul+Add -> GEMM fusion for performance boost#1506
snnn merged 1 commit into
microsoft:masterfrom
fdwr:user/dwayner/MatMulAddToGemmFusion

Conversation

@fdwr

@fdwr fdwr commented Jul 26, 2019

Copy link
Copy Markdown
Contributor

Description:
Enable float16 MatMul+Add -> GEMM fusion, which is currently blocked by a slightly too restrictive type check. The fusion does actually work correctly with float16 on both DirectML GPU and CPU with the check loosened.

Motivation and Context

  • The current 'if' check misses a measurable perf improvement opportunity for float16 models such as EmotionFerPlus. This perf issue was brought up by an external IHV while profiling on their hardware.
  • This is tested against our ONNX conformance suite and WinML model zoo.
  • MatMul + Add -> GEMM fusion not performed for float16 #1494 (related OS bug #22555607)

@fdwr
fdwr requested a review from a team as a code owner July 26, 2019 00:21
@hariharans29

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 22 pipeline(s).

Comment thread onnxruntime/core/optimizer/matmul_add_fusion.cc
@fdwr

fdwr commented Jul 29, 2019

Copy link
Copy Markdown
Contributor Author

@hariharans29 What do I need to do to kick start the test? Just type "/azp run" like you did? I looked at the failing Linux test, but it seems an unrelated error to my change:

Performing Test HAS_UNUSED_BUT_SET_VARIABLE - Failed ... CMake Error: The following variables are used in this project, but they are set to NOTFOUND. Please set them or make sure they are set and tested correctly in the CMake files: neural-networks-lib

@snnn
snnn merged commit cf73f63 into microsoft:master Jul 29, 2019
@snnn

snnn commented Jul 29, 2019

Copy link
Copy Markdown
Contributor

Hi @fdwr, It's a known failure. Please ignore it. CC @pranavsharma

@linkerzhang

Copy link
Copy Markdown
Contributor

android ci pipeline is not a "required" for now.

@fdwr
fdwr deleted the user/dwayner/MatMulAddToGemmFusion branch July 29, 2019 23:01
yuslepukhin pushed a commit that referenced this pull request Mar 17, 2026
## Describe your changes
Tidy up docs
## Checklist before requesting a review
- [ ] Add unit tests for this change.
- [ ] Make sure all tests can pass.
- [ ] Update documents if necessary.
- [ ] Lint and apply fixes to your code by running `lintrunner -a`
- [ ] Is this a user-facing change? If yes, give a description of this
change to be included in the release notes.
- [ ] Is this PR including examples changes? If yes, please remember to
update [example
documentation](https://github.com/microsoft/Olive/blob/main/docs/source/examples.md)
in a follow-up PR.

## (Optional) Issue link
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.

6 participants