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.

Sign up for GitHub

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

Jump to bottom

Fixes for clean run of backend with parallelism enabled #2685

Merged
merged 9 commits into from
Jan 24, 2024
Merged

Fixes for clean run of backend with parallelism enabled #2685

merged 9 commits into from
Jan 24, 2024

Conversation

AlexandreEichenberger
Copy link
Collaborator

I recently became aware of how to run check-onnx-backend with -parallel enabled on Linux-based platforms. It exposed a few bugs that are fixed in this PR. It now run cleanly.

OMP_NUM_THREADS=6 TEST_NOFLOAT16=true ONNX_MLIR_FLAGS="-O3 -mcpu=z16 -parallel" make -j 12 check-onnx-backend
[...]
bringing up nodes...
                                                                                                                                                                                                                                                                   [100%]
538 passed, 2268 skipped in 185.14s (0:03:05)
[100%] Built target check-onnx-backend
....

One of the issue is that `alloca_scope` is a bit fragile for some tests, but running `canonicalize` shortly after bufferization removes redundant copies, so that work well. Another other issue is that we were attempting to parallelize output loops for ops generating scalar output, so parallelism is now disabled for such loops.

This PR builds on #2684.

@AlexandreEichenberger
migrate adding alloca_scope to scf passes
6e0c85d 
Signed-off-by: Alexandre Eichenberger <alexe@us.ibm.com>
@AlexandreEichenberger
add test
6606cf5 
Signed-off-by: Alexandre Eichenberger <alexe@us.ibm.com>
@AlexandreEichenberger
format and comments
5bc109f 
Signed-off-by: Alexandre Eichenberger <alexe@us.ibm.com>
@AlexandreEichenberger
remove old unsued affine pass
b0a3c17 
Signed-off-by: Alexandre Eichenberger <alexe@us.ibm.com>
@AlexandreEichenberger
update
2a1a8ae
@AlexandreEichenberger
fixes for passing backend test with parallel on
2efa0b2 
Signed-off-by: Alexandre Eichenberger <alexe@us.ibm.com>
@AlexandreEichenberger
delete pass that was removed
ce7ae58 
Signed-off-by: Alexandre Eichenberger <alexe@us.ibm.com>
@AlexandreEichenberger
Merge branch 'main' into parallel-v7
c5001d7
@AlexandreEichenberger
Copy link
Collaborator Author

@tungld Note that this does not turn on parallelism by default or test it yet. It is just to enable a clean run

tungld
tungld approved these changes Jan 24, 2024
View reviewed changes
Copy link
Collaborator

@tungld tungld left a comment

Choose a reason for hiding this comment

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

LGTM.

I though I reviewed this yesterday, but not :(

@AlexandreEichenberger AlexandreEichenberger merged commit 58f3746 into onnx:main Jan 24, 2024
8 checks passed
@jenkins-droid
Copy link
Collaborator

Jenkins Linux s390x Build #13927 [push] Fixes for clean run of b... started at 10:45

@jenkins-droid
Copy link
Collaborator

Jenkins Linux amd64 Build #13900 [push] Fixes for clean run of b... started at 09:45

@jenkins-droid
Copy link
Collaborator

Jenkins Linux ppc64le Build #12924 [push] Fixes for clean run of b... started at 10:53

@jenkins-droid
Copy link
Collaborator

Jenkins Linux amd64 Build #13900 [push] Fixes for clean run of b... passed after 1 hr 3 min

@jenkins-droid
Copy link
Collaborator

Jenkins Linux s390x Build #13927 [push] Fixes for clean run of b... passed after 1 hr 23 min

@jenkins-droid
Copy link
Collaborator

Jenkins Linux ppc64le Build #12924 [push] Fixes for clean run of b... passed after 1 hr 53 min

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tungld tungld tungld approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AlexandreEichenberger @jenkins-droid @tungld