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

Option to not emit the full MLIR (only emit .tmp file) #2997

Merged
merged 5 commits into from
Nov 7, 2024

Conversation

imaihal
Copy link
Collaborator

@imaihal imaihal commented Nov 1, 2024

When emitting MLIR by the option such as --EmitONNXIRetc., there are two versions of IR, <name>.onnx.mlir and <name>.tmp . Since the constant values are embedded in <name>.onnx.mlir, we got memory and disk pressure especially in large models. In mistral-7b model, the MLIR size is about 50GB.
This PR provides an option to not emit full MLIR(<name>.onnx.mlir). This option works with emitting MLIR options such as --EmitONNXIR and --EmitMLIR.

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. It would be an useful option. Thanks!

Could you add a lit test for this option if you don't mind?

@@ -281,6 +282,13 @@ static llvm::cl::opt<bool, true> printIROpt("printIR",
llvm::cl::desc("Print the IR to stdout:."), llvm::cl::location(printIR),
llvm::cl::init(false), llvm::cl::cat(OnnxMlirOptions));

static llvm::cl::opt<bool, true> emitElidedIROnlyOpt("emitElidedIROnly",
Copy link
Collaborator

Choose a reason for hiding this comment

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

emitElidedIROnly is a bit confusing in the sense that it looks like other EmitONNXIR, EmitMLIR, ... How about doNotEmitFullMLIRCode (--do-not-emit-full-mlir-code)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated the option name. Thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, added the lit test.

@imaihal imaihal changed the title Option to emit only IR without constants (.tmp file). Option to not emit full MLIR (only emit .tmp file) Nov 7, 2024
@imaihal imaihal changed the title Option to not emit full MLIR (only emit .tmp file) Option to not emit the full MLIR (only emit .tmp file) Nov 7, 2024
@imaihal imaihal merged commit b856fc5 into onnx:main Nov 7, 2024
7 checks passed
@imaihal imaihal deleted the emit_elided branch November 7, 2024 06:13
@jenkins-droid
Copy link
Collaborator

Jenkins Linux ppc64le Build #14981 [push] Option to not emit the f... started at 01:27

@jenkins-droid
Copy link
Collaborator

Jenkins Linux s390x Build #15955 [push] Option to not emit the f... started at 01:14

@jenkins-droid
Copy link
Collaborator

Jenkins Linux amd64 Build #15952 [push] Option to not emit the f... started at 00:14

@jenkins-droid
Copy link
Collaborator

Jenkins Linux amd64 Build #15952 [push] Option to not emit the f... passed after 1 hr 20 min

@jenkins-droid
Copy link
Collaborator

Jenkins Linux s390x Build #15955 [push] Option to not emit the f... passed after 1 hr 58 min

@jenkins-droid
Copy link
Collaborator

Jenkins Linux ppc64le Build #14981 [push] Option to not emit the f... passed after 2 hr 21 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