oneapi-src / oneDAL

oneAPI Data Analytics Library (oneDAL)
https://software.intel.com/en-us/oneapi/onedal
Apache License 2.0
617 stars 214 forks source link

MAINT: Remove unneeded .def files #2974

Closed david-cortes-intel closed 6 days ago

david-cortes-intel commented 1 week ago

Description

This PR removes BLAS and LAPACK functions from the .def files that export them in oneDAL's .dlls for windows.

After the move to oneMKL, these shouldn't be necessary anymore.


PR should start as a draft, then move to ready for review state after CI is passed and all applicable checkboxes are closed. This approach ensures that reviewers don't spend extra time asking for regular requirements.

You can remove a checkbox as not applicable only if it doesn't relate to this PR in any way. For example, PR with docs update doesn't require checkboxes for performance while PR with any change in actual code should have checkboxes and justify how this code change is expected to affect performance (or justification should be self-evident).

Checklist to comply with before moving PR from draft:

PR completeness and readability

Testing

Performance

Not applicable.

david-cortes-intel commented 1 week ago

/intelci: run

david-cortes-intel commented 1 week ago

/intelci: run

david-cortes-intel commented 1 week ago

Looks like empty .def files (with just "export") were indeed needed by some make scripts:

make: *** No rule to make target '__work/md/lnx32e/threading_dynamic/libonedal_thread_link.def', needed by '__work/md/lnx32e/daal/lib/libonedal_thread.so'.  Stop.

@Alexandr-Solovev Any idea of why that might be the case?

Alexandr-Solovev commented 1 week ago

Looks like empty .def files (with just "export") were indeed needed by some make scripts:

make: *** No rule to make target '__work/md/lnx32e/threading_dynamic/libonedal_thread_link.def', needed by '__work/md/lnx32e/daal/lib/libonedal_thread.so'.  Stop.

@Alexandr-Solovev Any idea of why that might be the case?

Interesting, I will take a look

Alexandr-Solovev commented 1 week ago

@david-cortes-intel I locally fixed it, not sure that all tests are passed, but it builds successfully Firstly I removed this: https://github.com/oneapi-src/oneDAL/blob/main/makefile#L843 https://github.com/oneapi-src/oneDAL/blob/main/makefile#L844 And also removed $(THR.tmpdir_y)/$(thr_tbb_y:%.$y=%_link.def)from this line https://github.com/oneapi-src/oneDAL/blob/main/makefile#L848 Looks like it searches .def files for threading and returns error. Can you apply this changes and check it? upd: I also removed https://github.com/oneapi-src/oneDAL/blob/main/makefile#L492 and oneDAL/cpp/daal/src/algorithms/export_win32e.def but it probably affects windows, didnt test it

david-cortes-intel commented 1 week ago

/intelci: run

david-cortes-intel commented 6 days ago

Looks like that worked. All CI jobs are passing now.