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

[ROCm] enable hipGraph #18382

Merged
merged 3 commits into from
Jan 23, 2024
Merged

Conversation

jeffdaily
Copy link
Contributor

@jeffdaily jeffdaily commented Nov 9, 2023

This ports the cudaGraph support from the CUDA EP to the ROCM EP's hipGraph.

@tianleiwu
Copy link
Contributor

Update the CUDA Graph unit test to cover hipGraph?

@cloudhan cloudhan self-assigned this Nov 15, 2023
@cloudhan
Copy link
Contributor

@hariharans29 @tianleiwu Do we have any test around CUDA graph? If yes, maybe some #ifdefs should be taken care.

@jeffdaily
Copy link
Contributor Author

Update the CUDA Graph unit test to cover hipGraph?

Is this the only one?

onnxruntime/onnxruntime/test/python/onnxruntime_test_python_cudagraph.py

@tianleiwu
Copy link
Contributor

tianleiwu commented Nov 16, 2023

Update the CUDA Graph unit test to cover hipGraph?

Is this the only one?

onnxruntime/onnxruntime/test/python/onnxruntime_test_python_cudagraph.py

Please make sure this test can cover hipGraph. The others can be verified by build pipeline.

@tianleiwu
Copy link
Contributor

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, Linux QNN CI Pipeline

@tianleiwu
Copy link
Contributor

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows ARM64 QNN CI Pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, ONNX Runtime React Native CI Pipeline, Windows x64 QNN CI Pipeline

Copy link

Azure Pipelines successfully started running 9 pipeline(s).

1 similar comment
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@cloudhan
Copy link
Contributor

@jeffdaily @hariharans29 @tianleiwu Should this merge as is?

@jeffdaily jeffdaily force-pushed the rocm_enable_hipgraph_upstream branch from e3dff16 to f3b46e4 Compare January 10, 2024 21:26
@jeffdaily
Copy link
Contributor Author

@jeffdaily @hariharans29 @tianleiwu Should this merge as is?

I think so. I just rebased against latest upstream main. I also ported two cudagraph tests from the shared lib option. They pass for me when I enabled the shared lib build.

Note: Google Test filter = *graph*
[==========] Running 3 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 3 tests from CApiTest
[ RUN      ] CApiTest.basic_cuda_graph
[       OK ] CApiTest.basic_cuda_graph (956 ms)
[ RUN      ] CApiTest.hip_graph_with_shape_nodes
2024-01-10 21:27:37.244050431 [W:onnxruntime:, inference_session.cc:1754 Initialize] This model has shape massaging nodes that will execute on CPU. Use the CUDA/HIP Graph feature with caution. As long as the intermediate shapes produced in the model using the representative input used to capture the CUDA/HIP graph, will match the shapes produced in the model for other inputs of the same shape as the representative input (common case), it is safe to use the CUDA/HIP Graph feature.
2024-01-10 21:27:37.244161810 [W:onnxruntime:, session_state.cc:1166 VerifyEachNodeIsAssignedToAnEp] Some nodes were not assigned to the preferred execution providers which may or may not have an negative impact on performance. e.g. ORT explicitly assigns shape related ops to CPU to improve perf.
2024-01-10 21:27:37.244177249 [W:onnxruntime:, session_state.cc:1168 VerifyEachNodeIsAssignedToAnEp] Rerunning with verbose output on a non-minimal build will show node assignments.
[       OK ] CApiTest.hip_graph_with_shape_nodes (24 ms)
[ RUN      ] CApiTest.session_options_graph_optimization_level
[       OK ] CApiTest.session_options_graph_optimization_level (0 ms)
[----------] 3 tests from CApiTest (980 ms total)

[----------] Global test environment tear-down
[==========] 3 tests from 1 test suite ran. (980 ms total)
[  PASSED  ] 3 tests.

@cloudhan
Copy link
Contributor

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, Linux QNN CI Pipeline

@cloudhan
Copy link
Contributor

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows ARM64 QNN CI Pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, ONNX Runtime React Native CI Pipeline, Windows x64 QNN CI Pipeline

Copy link

Azure Pipelines successfully started running 9 pipeline(s).

1 similar comment
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@tianleiwu
Copy link
Contributor

@cloudhan, Could you take a look and see whether it is good to merge now?

@cloudhan
Copy link
Contributor

It has always been ready, but the required pipeline is preventing the PR from being merged...

tianleiwu
tianleiwu previously approved these changes Jan 20, 2024
@tianleiwu
Copy link
Contributor

Lint / Python format pipeline failed. Need to merge latest change from main, then run lintrunner -a.

@jeffdaily
Copy link
Contributor Author

@tianleiwu @cloudhan I have rebased against latest main and ran lintrunner -a.

@tianleiwu
Copy link
Contributor

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, Linux QNN CI Pipeline

@tianleiwu
Copy link
Contributor

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows ARM64 QNN CI Pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, ONNX Runtime React Native CI Pipeline, Windows x64 QNN CI Pipeline

Copy link

Azure Pipelines successfully started running 9 pipeline(s).

1 similar comment
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@cloudhan cloudhan merged commit b2aec41 into microsoft:main Jan 23, 2024
67 checks passed
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.

4 participants