MAINT: Reuse common routines in covariance #3227
Conversation
|
@david-cortes-intel can you explain why this won't need performance benchmarks? It looks like the vectorization hints are switched to an omp pragma. |
|
/intelci: run |
That happens only in the 'ref' headers. Otherwise, it gets replaced with calls to MKL. The builds with reference implementation use GCC which either way doesn't support those same pragmas. |
|
if you want to enable this for only gcc i recommend leaving comments in the code as such. wouldn't these changes then pertain to any build not using MKL regardless of the compiler? |
I'm not understanding that part. The change is from using compiler hints towards using functions from the 'service' headers. When building with MKL, the functions get taken from MKL regardless of compiler, while when building without MKL, the functions are generated by the compiler, using |
|
@icfaust I've now reverted the changes on the sections that had the pragma for aligned data, although I'm pretty sure MKL would be able to make the same optimization. |
|
/intelci: run |
| @@ -214,6 +232,12 @@ struct RefMath<float, cpu> | |||
| for (SizeType i = 0; i < n; ++i) out[i] = sqrt(in[i]); | |||
| } | |||
|
|
|||
| static void vInvSqrtI(SizeType n, const float * a, const SizeType inca, float * b, const SizeType incb) | |||
There was a problem hiding this comment.
For this is inca and incb usually 1? If so there might be a better way writing this so the compiler optimizes the code.
There was a problem hiding this comment.
MKL has different variants of these functions. There is a variant in which they are both fixed at 1, but this PR uses it for a case where one of them is not equal to 1.
|
I agree with @icfaust and think that these changes require the benchmarks run. We can have some assumptions, but they really need to be proved by actual performance data. |
Description
This PR simplifies some parts of the covariance implementation on CPU to reuse common routines from either MKL or from the rest of daal. This should slightly simplify future work on compiler pragmas.
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