Skip to content

Filtered_kernel: Optimisation of side_of_oriented_sphere_3()#9546

Open
afabri wants to merge 4 commits into
CGAL:mainfrom
afabri:Filtered_kernel-side_of_oriented_sphere-GF
Open

Filtered_kernel: Optimisation of side_of_oriented_sphere_3()#9546
afabri wants to merge 4 commits into
CGAL:mainfrom
afabri:Filtered_kernel-side_of_oriented_sphere-GF

Conversation

@afabri

@afabri afabri commented Jun 29, 2026

Copy link
Copy Markdown
Member

Summary of Changes

This PR is the starting point to find out if and how we can accelerate the static filter of Side_of_oriented_sphere_3.
It is the first item in vtune in bottom up view when constructing a Delaunay_triangulation_3.
Running an executable that computes a triangulation has enormous fluctuations in performance. This PR has a standalone program that reads a point cloud into a std::vector and performs N times a loops over the vector, taking four consecutive points, and performing the test on the next Q points. The point cloud file name, as well as N and Q are parameters of the standalone.

The static filter first computes 12 absolute values, and then computes 3 times the max of groups of four absolute values.
We use CGAL::abs(double) where we already use std::fabs() instead of std::abs().

In the code we have the macro CGAL_USE_SSE2_MAX to enable an implementation using SSE2 for computing max.
We also have a macro CGAL_USE_SSE2_FABS for the usage of SSE2 inside CGAL::abs(double) instead of using std::fabs(). The macros are by default undefined.

Our code presented to chatgpt, it recommends to replace by

double maxx = (std::max)({std::abs(ptx), std::abs(qtx), std::abs(rtx), std::abs(stx)});
double maxy = (std::max)({std::abs(pty), std::abs(qty), std::abs(rty), std::abs(sty)});
double maxz = (std::max)({std::abs(ptz), std::abs(qtz), std::abs(rtz), std::abs(stz)});

and to hope that the compiler vectorizes the code. It suggests to check the generated assembler.
Also our functions using SSE2 presented to chatgpt, you get quite some remarks that for
just computing the abs of a single double there is nothing to gain, and if doing
several abs and max computations the doubles should be in an array and not just individual double variables.

This PR is WIP and tries to systematically benchmark, adding as another dimension the compiler, and the operating system.

Here is the table for running once for Q queries, and 10 times forQ/10 queries, for VC++ as well as for clang/Windows.

Q 100 1000 10000
1 * Q 1.0 8.9 91
10 * Q/10 1.4 10.3 106

We next try the suggested code, only for the 1 * Q scenario, once with std::abs() once with std::fabs() and VC++

Q 100 1000 10000
std::abs() 2.2 20.2 204
std::fabs() 2.1 20.3 207

and this time for clang 19.1.1 on Windows

Q 100 1000 10000
std::abs()
std::fabs() 3.7 37 472

Release Management

  • Affected package(s):
  • Issue(s) solved (if any): fix #0000, fix #0000,...
  • Feature/Small Feature (if any):
  • Link to compiled documentation (obligatory for small feature) wrong link name to be changed
  • License and copyright ownership:


#include <CGAL/Profile_counter.h>
#include <CGAL/Filtered_kernel/internal/Static_filters/Static_filter_error.h>
#include <immintrin.h>

@sloriot sloriot Jul 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

macOS does not like it. See here

return _mm_cvtsd_f64(m);
}

inline __m256d abs4(__m256d v)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/mnt/testsuite/include/CGAL/Filtered_kernel/internal/Static_filters/Side_of_oriented_sphere_3.h:33:30: warning: AVX vector return without AVX enabled changes the ABI [-Wpsabi]
   33 | inline __m256d abs4(__m256d v)
      |                              ^

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.

2 participants