Make the project compatible with FetchContent and find_package #200
Conversation
AndreyPavlenko
force-pushed
the
pkg
branch
5 times, most recently
from
3535da3
to
ae30041
Compare
| @@ -5,6 +5,7 @@ | |||
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | |||
| // | |||
| //===----------------------------------------------------------------------===// | |||
| #ifdef GC_HAS_ONEDNN_DIALECT | |||
There was a problem hiding this comment.
Can we not include the file in the build instead?
There was a problem hiding this comment.
Yes, but in this case I'm getting the error:
LLVM's build system enforces that all source files are added to a build target
I could suppress the error, but I'm not sure if it's a better solution.
There was a problem hiding this comment.
We can conditionally add it to the global property that forms an appropriate library (be that gcpasses or a separate one, no matter). I had a similar problem in #205
CMakeLists.txt
Outdated
| GcCpuRuntime | ||
| GcGpuPasses | ||
| GcInterface | ||
| GcJitWrapper | ||
| GcPasses | ||
| GcUtilsIR | ||
| MLIRCPURuntimeDialect | ||
| MLIRCPURuntimeTransforms | ||
| MLIRLinalgx | ||
| MLIRMicrokernel |
There was a problem hiding this comment.
So these are to cover the static scenario. I think this should eventually be passed through via a global property to be flexible.
There was a problem hiding this comment.
Do you mean so that each module adds its targets to the global property?
There was a problem hiding this comment.
Yup, this is just a thought.
CMakeLists.txt
Outdated
| option(GC_ENABLE_TEST "Build the tests" ON) | ||
| option(GC_ENABLE_TEST_DNNL "Build the dnnl tests" ${GC_ENABLE_DNNL}) | ||
| option(GC_ENABLE_TEST_MLIR "Build the mlir tests" ON) | ||
| option(GC_ENABLE_OPT "Build gc-opt" ON) |
There was a problem hiding this comment.
Can be more generic, i.e. GC_ENABLE_TOOLS.
to simplify integration with thirdparty projects. - Use the Gc prefix in the target names. - Added a separate shared library for oneDNN integration - GcDnnl. - The GcDnnl library and the OneDNNGraphDialect are not built if GC_ENABLE_DNNL=OFF. - Added new build options: GC_ENABLE_TEST(renamed), GC_ENABLE_TEST_DNNL, GC_ENABLE_TEST_MLIR, GC_ENABLE_OPT. - Renamed the GC_USE_GPU option to GC_ENABLE_GPU to be consistent with other options. - Export the OpenMP flags to resolve the "JIT session error: Symbols not found: __kmpc_...". - Minor compile.sh script enhancements.
AndreyPavlenko
force-pushed
the
pkg
branch
11 times, most recently
from
f8dfaac
to
0f20408
Compare
|
I would suggest splitting the PR into several PRs which match items in the list, it just very hard to follow when we have all the items mixed. At least I suggest splitting it into two parts: essential and non-essential like renamings. |
Sorry for that. Initially it was much smaller... |
to simplify integration with thirdparty projects.