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

Move makefiles into x86_64 target directory #2604

Closed

Conversation

keeranroth
Copy link
Contributor

It would be nice to be able to build oneDAL on different CPU targets. To make it easier to develop build files for different targets, we move the existing build files into dev/make/x86_64.

The top level makefile takes a TARGET parameter, which at the moment can only be x86_64, and defaults to this. This means that the build commands do not change with this. Any future changes to the builds need to be done in the dev/make/x86_64 directory.

Changes proposed in this pull request:

  • Take the existing makefiles and put these in an x86_64 subdirectory
  • Top level makefile by default includes the x86_64 subdirectory, so no changes to build procedure

@inteldimitrius
Copy link
Contributor

/intelci: run

@inteldimitrius
Copy link
Contributor

Hi @keeranroth , could you please rebase this branch onto latest master to avoid merging conflicts?

It would be nice to be able to build oneDAL on different CPU targets. To
make it easier to develop build files for different targets, we move the
existing build files into dev/make/x86_64.

The top level makefile takes a TARGET parameter, which at the moment can
only be x86_64, and defaults to this. This means that the build commands
do not change with this. Any future changes to the builds need to be
done in the dev/make/x86_64 directory.
After the rebase, make sure that the makefile changes from the dev
branch are applied to the moved files.
@keeranroth
Copy link
Contributor Author

Hi @inteldimitrius. I rebased the commit, and added a commit to show that the changes from the development branch have been applied to the moved files

@inteldimitrius
Copy link
Contributor

Cool! Thanks! Let's wait CI checks!

@inteldimitrius
Copy link
Contributor

/intelci: run

Taking the same copyright text as in the top level makefile, and
changing the year to 2023.
@ethanglaser
Copy link
Contributor

http://intel-ci.intel.com/ee92aa44-4015-f116-8512-a4bf010d0e2e

@ethanglaser
Copy link
Contributor

/intelci: run

@ethanglaser
Copy link
Contributor

/intelci: run

@napetrov
Copy link
Contributor

napetrov commented Dec 6, 2023

In current form this would assume that there would be ether different implementation of makefiles or there would be lot of code duplication with makes.

I guess there is only 20-30% of code considered to be platform specific, so from this perspective there would be better to just separate arch specific parts while keep single core.

But the bigger question - do we want this to be make? Like longerterm plan is to migrate from make to bazel and abandon make.
Would it make sense to develop with focus on bazel instead?

I guess there are some pieces missed there but it should be fairly simple to get this enabled there

@keeranroth
Copy link
Contributor Author

In current form this would assume that there would be ether different implementation of makefiles or there would be lot of code duplication with makes.

I guess there is only 20-30% of code considered to be platform specific, so from this perspective there would be better to just separate arch specific parts while keep single core.

But the bigger question - do we want this to be make? Like longerterm plan is to migrate from make to bazel and abandon make. Would it make sense to develop with focus on bazel instead?

I guess there are some pieces missed there but it should be fairly simple to get this enabled there

Agreed that the current approach can be cleaned up, and ideally we can get something in that is future-proof. To start with I wanted to put the easiest option up so that some discussion can be opened up.

The question is then, what the right path forward would be. I see the following options:

  1. Bazel build is close to being ready, in which case drop this commit in favour of updating bazel to allow for (micro-)architecture specific build options.
  2. Bazel is close to being ready, but not for at least a month. In this case, I would say that the change as it is would be good to put in, as it would allow me to get some other changes that I have locally put up for a pull request, and then we can look at the bazel build when that is a little more mature
  3. Not sure when bazel build is ready. Makefiles are going to be around for a while, in which case I'll update the makefiles to take out (micro-)architecture specific parts only

So the question is then, how close is the bazel build to being ready, and which one of options 1-3 above would be the best approach? (unless there is a better option 4 😄 )

@napetrov
Copy link
Contributor

napetrov commented Dec 7, 2023

@keeranroth
In general bazel can be used to build library already for linux. The main thing that is missing currently in bazel is that it doesn't create release structure similar to one that make does, so it just put libs + includes.

Also there is no working bazel solution on windows yet - but i guess this is not a concern in your case.

I would imagine that we would be ready with bazel for linux part in Q1 and Windows closer to the end of the year.

But the point is - technically we can split in time build system usage and use make for x86 build now but bazel for new arches.

However this would depend on overall work requeued for make file modification - if this is relatively low work - then we can do make changes and do switch to bazel later.

Hope this make sense

@keeranroth
Copy link
Contributor Author

@napetrov That does make sense.

I have a few changes that could go up without updates to the build system, actually, which are required for building on multiple arches anyway. There are a few places where x86-specifec code paths are called. I'll try putting some of these up first, and when I get to the end of that stack, I'll have a look at how far the bazel build has come, and re-evaluate whether updating the make or going for the bazel makes more sense.

I've marked the pull request as draft for now, as it could come in useful again in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please consider just renamin the file

@keeranroth
Copy link
Contributor Author

Closing, as this is superseded by #2614

@keeranroth keeranroth closed this Feb 26, 2024
@keeranroth keeranroth deleted the dev/keeranroth/x86-make-dir branch March 13, 2024 12:51
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.

6 participants