Skip to content

Conversation

@dor-forer
Copy link
Collaborator

Describe the changes in the pull request

A clear and concise description of what the PR is solving.

Which issues this PR fixes

  1. MOD-9011

Main objects this PR modified

  1. Added the download and extract armpl to the install script.
  2. Added to spaces' cmakelists the include and find lib of the armpl.
  3. Added support of Armpl with Blas optimization for IP and L2 spaces.
  4. Benchmarks for Neon, SVE, and SVE2 were added (depending on the compiling options).
  5. Unit tests were added for Arm.

Mark if applicable

  • This PR introduces API changes
  • This PR introduces serialization changes

alonre24
alonre24 previously approved these changes Mar 19, 2025

namespace spaces {

#include "implementation_chooser.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Irrelevant but harmless


namespace spaces {

#include "implementation_chooser.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Irrelevant but harmless

Comment on lines 54 to 59
if ((chunk) == 1) { \
/* Handle the case where chunk is 0 */ \
__ret_dist_func = func<1>; \
} else { \
switch ((dim) % (chunk)) { CASES##chunk(func) } \
} \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's revert these changes

Comment on lines 48 to 49
// chunk == 1 means that there's no use of the residual, and we can use the function
// directly.
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove

@dor-forer dor-forer requested a review from GuyAv46 March 24, 2025 14:28
ASSERT_EQ(arch_opt_func, Choose_FP32_L2_implementation_AVX(dim))
<< "Unexpected distance function chosen for dim " << dim;
ASSERT_EQ(baseline, arch_opt_func(v, v2, dim)) << "AVX with dim " << dim;
ASSERT_EQ(alignment, expected_alignment(256, dim)) << "AVX with dim " << dim;
Copy link
Collaborator

Choose a reason for hiding this comment

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

revert

ASSERT_EQ(arch_opt_func, Choose_FP32_L2_implementation_SSE(dim))
<< "Unexpected distance function chosen for dim " << dim;
ASSERT_EQ(baseline, arch_opt_func(v, v2, dim)) << "SSE with dim " << dim;
ASSERT_EQ(alignment, expected_alignment(128, dim)) << "SSE with dim " << dim;
Copy link
Collaborator

Choose a reason for hiding this comment

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

revert

GuyAv46
GuyAv46 previously approved these changes Mar 24, 2025
@dor-forer dor-forer added this pull request to the merge queue Mar 25, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 25, 2025
@dor-forer dor-forer requested a review from GuyAv46 March 25, 2025 08:46
@dor-forer
Copy link
Collaborator Author

@GuyAv46 @alonre24 Added support for macos

@dor-forer dor-forer requested a review from alonre24 March 25, 2025 08:46
@dor-forer dor-forer closed this Mar 25, 2025
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.

4 participants