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

[bench] enable mlir benchmark #106

Merged
merged 115 commits into from
Aug 15, 2024
Merged

[bench] enable mlir benchmark #106

merged 115 commits into from
Aug 15, 2024

Conversation

xurui1995
Copy link
Contributor

@xurui1995 xurui1995 commented May 29, 2024

Enable benchmark and tuning from python
trace: #57

  • running gc-cpu-pipeline in python
  • bench a MLIR file with path
  • generate and bench mlp pattern with options
  • implement tuner (will have a new PR)
  • support tensor or memref with different data types

There are still some features that will be implemented when integrated into the PR of benchgc #161 ,such as data filling, customize pass pipeline.

Please read this if you need to use this benchmark and tuner tools.
https://github.com/intel/graph-compiler/blob/xurui/add_benchmark/tools/README.md

@xurui1995 xurui1995 added the WIP work in progress label May 29, 2024
xurui1995 and others added 4 commits July 26, 2024 09:41
Co-authored-by: Petr Kurapov <petr.a.kurapov@intel.com>
Co-authored-by: Petr Kurapov <petr.a.kurapov@intel.com>
@xurui1995
Copy link
Contributor Author

I still find the purpose of the tool puzzling. Why does it rely on FileCheck to run IR tests? What does it add on top of just running FileCheck via lits?

The FileCheck in python is for testing if the bindings work, this is also how MLIR tests their bindings, and it's not the purpose of this bench tool.

Seems you put two features together:

  1. Add python-binding support for GC downstream dialect and passes.
  2. Enable benchmark tool on top of python-binding.

Would you please split them into separate PRs for better code-review?

linalgx bindings code was removed now

@xurui1995
Copy link
Contributor Author

@kurapov-peter @Menooker @ZhennanQin @ciyongch hi, could you help to review the changes since last time? @WangJialei-A 's validation PR #161 is blocked now, let's get this merged first.

@@ -0,0 +1,2 @@
MLIR_C_RUNNER_UTILS = "@MLIR_DIR@/../../libmlir_c_runner_utils.so"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a good solution for Windows platform. Please try platform platform-independent solution.

Copy link

@Menooker Menooker Aug 12, 2024

Choose a reason for hiding this comment

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

I agree with @ZhennanQin . Actually there are some examples in our repo for python binding .py.in to generate file names with OS-specific extension names. Also @MLIR_DIR@/../../ seems fragile. I think we have already exposed this directory somewhere in .py.in ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my laptop has a problem now, I will fix this as soon as it returns to normal. I am trying to change it with os.path to implement platform-independent.
@Menooker we do have that exposed but it was in the test dir for lit config, I can not use that from here.
I also noticed that we have something like the following in other .py.in, should change it to the solution with os.path ?

lit_config.load_config(config, "@PROJECT_SOURCE_DIR@/test/mlir/test/lit.cfg.py")
...
config.substitutions.append(("%gclibdir", config.gc_obj_root + "/lib/"))
...

@@ -0,0 +1,8 @@
import os

MLIR_C_RUNNER_UTILS = os.path.normpath(
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better to just register a folder path here, maybe MLIR_SHARED_LIB_PATH, and then in the graph_compiler.py file, you can search the lib name in an OS independent way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed this with similar way in lit.site.cfg.py, but we can not get the SHLIBEXT value which defined in configure_lit_site_cfg, according to the source code we can use LTDL_SHLIB_EXT
https://github.com/llvm/llvm-project/blob/109b50808f72c228518766c3b384dd14e0dcf4ee/llvm/cmake/modules/AddLLVM.cmake#L1841-L1854

https://github.com/llvm/llvm-project/blob/109b50808f72c228518766c3b384dd14e0dcf4ee/llvm/cmake/modules/HandleLLVMOptions.cmake#L233

Choose a reason for hiding this comment

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

We can set mlir_runner_utils_dir to @MLIR_RUNNER_UTILS_DIR@ for our lit.site.cfg.py.in. Can we use that cmake variable too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried and we can not get the @MLIR_RUNNER_UTILS_DIR@ from here, not in the same scope.

python/config.py.in Outdated Show resolved Hide resolved
@xurui1995 xurui1995 merged commit c744f5b into main Aug 15, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants