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

Add all-in-one driver for compile-and-execute #97

Merged
merged 47 commits into from
Jun 11, 2024
Merged

Conversation

Menooker
Copy link
Contributor

For usage see unittest.

Also add a wrapper for mlir::ExecutionEngine for further developing of constant cache.

@Menooker
Copy link
Contributor Author

Dependency #79 #75

@Menooker Menooker changed the base branch from main to yijie/pipeline May 23, 2024 09:09
@Menooker
Copy link
Contributor Author

I think it would be nice to use the same code style for the entire code. Currently, one part of the code is in the CamelCase, another - in snake_case.

I agree with you. The code was copied from legacy. Now I have updated the coding style to CamelCase.

}
compute = *expectCompute;
}
llvm::ArrayRef<uint64_t> foldBufferIds;

Choose a reason for hiding this comment

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

Better to unify the element dtypes here with the Globals in MLIR module. "__num_orig_num_args", "__fold_args" and "__compute_args" are i32, and "__fold_buffer_ids" is i64.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

__fold_buffer_ids is the UID of a buffer, so we use i64. __num_orig_num_args and others are i32 because we won't meet too many args in the same function if it is reasonable. We use i32 for __fold_args and __compute_args for performance because it save half of space and cache (!!) at each execution time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need another one tests folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The folder tests is for MLIR-related FileCheck-based tests and unittests is for MLIR-related C++ gtests. The convention of tests and unittests is borrowed from MLIR upstream - they use these folders too.
I have several pending PRs to put the C++-based gtests in unittests folder.

@Menooker Menooker changed the base branch from yijie/pipeline to main May 29, 2024 03:33
@Menooker
Copy link
Contributor Author

Menooker commented May 30, 2024

Hi, all! I have removed the constant cache related code. This PR now only contains all-in-one ModuleOp to JitModule conversion. And it also provides a unified API to initialize the compiler (LLVM JIT, MLIR passes) and DialectRegistry.

The constant cache related code is moved to branch yijie/const_cache_jit.

AndreyPavlenko added a commit to AndreyPavlenko/graph-compiler that referenced this pull request Jun 9, 2024
@Menooker Menooker removed the WIP work in progress label Jun 11, 2024
compute(realargs.data());
}

// directly call compute(). args should be an array of void*. args[i] should
Copy link
Contributor

Choose a reason for hiding this comment

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

// -> ///

@Menooker Menooker merged commit 837696b into main Jun 11, 2024
4 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.

None yet

6 participants