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

Migrate adding alloca_scope to scf passes (for parallel with NNPA) #2684

Merged
merged 6 commits into from
Jan 22, 2024

Conversation

AlexandreEichenberger
Copy link
Collaborator

@AlexandreEichenberger AlexandreEichenberger commented Jan 19, 2024

Previous approach of adding alloca_scope at the affine level did not work for NNPA as additional passes of canonicalize were called between adding alloca_scope and handling of buffers when generating NNPA code.

This problem is fixed here by migrating that pass to work with scf and calling that pass just before handling buffers.

Once this is well established, we should probably remove the old pass at the affine level as its unlikely to be useful anymore.

With this fix, Roberta passes with NNPA:

CheckONNXModel.py -m roberta-base-11.onnx -r="-O3 -mcpu=z16 -maccel=NNPA" -a="--parallel" --shape-info=0:8x384 

> Reference command:  /workdir/onnx-mlir/build/Debug/../../utils/RunONNXModel.py --compile-args="-O3 -mcpu=z16 -maccel=NNPA" --save-ref=check-ref --shape-info=0:8x384 --seed=42 --model=roberta-base-11.onnx
>   Successfully ran the reference example, saved refs in "check-ref".

> Test command:  /workdir/onnx-mlir/build/Debug/../../utils/RunONNXModel.py --compile-args="-O3 -mcpu=z16 -maccel=NNPA --parallel" --load-ref=check-ref --verify=ref --verify-every-value --model=roberta-base-11.onnx
>   Successfully ran the test example and verified against "check-ref".

Signed-off-by: Alexandre Eichenberger <[email protected]>
Signed-off-by: Alexandre Eichenberger <[email protected]>
@AlexandreEichenberger AlexandreEichenberger changed the title Migrate adding alloca_scope to scf passes Migrate adding alloca_scope to scf passes (for parallel with NNPA) Jan 19, 2024
Signed-off-by: Alexandre Eichenberger <[email protected]>
Signed-off-by: Alexandre Eichenberger <[email protected]>
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.

@AlexandreEichenberger AlexandreEichenberger merged commit 592618f into onnx:main Jan 22, 2024
8 checks passed
@jenkins-droid
Copy link
Collaborator

Jenkins Linux s390x Build #13914 [push] Migrate adding alloca_sc... started at 10:13

@jenkins-droid
Copy link
Collaborator

Jenkins Linux ppc64le Build #12911 [push] Migrate adding alloca_sc... started at 10:22

@jenkins-droid
Copy link
Collaborator

Jenkins Linux amd64 Build #13887 [push] Migrate adding alloca_sc... started at 09:13

@jenkins-droid
Copy link
Collaborator

Jenkins Linux amd64 Build #13887 [push] Migrate adding alloca_sc... passed after 1 hr 10 min

@jenkins-droid
Copy link
Collaborator

Jenkins Linux s390x Build #13914 [push] Migrate adding alloca_sc... passed after 1 hr 35 min

@jenkins-droid
Copy link
Collaborator

Jenkins Linux ppc64le Build #12911 [push] Migrate adding alloca_sc... passed after 1 hr 55 min

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.

3 participants