Uh oh!
There was an error while loading. Please reload this page.
This repository was archived by the owner on Aug 20, 2026. It is now read-only.
Make half sqrt/min/max compile under nvcc - #5
Merged
Conversation
Under __CUDACC__ the half specializations failed to compile: - sqrt<half> called ::hsqrt, which is not visible at global scope in every CUDA/arch combination; compute in float and narrow back. - min<half>/max<half> compared the half values directly (a < b), which is ambiguous because half has multiple implicit conversions to built-in types; compare the already-computed float forms instead. Surfaced when the cuda-lib nvcc build began compiling translation units as CUDA (-x cu); no functional change on the CPU path (guarded by #ifdef __CUDACC__).
balbasty added a commit
that referenced
this pull request
Aug 9, 2026
…74) `MeshDist::build_tree` sorts the faces of a BVH node with std::sort(FaceIterator(faces.data, ...) + begin, FaceIterator(faces.data, ...) + end, cmp); `FaceIterator`'s constructor eagerly builds a `Face`, whose pointer constructor loaded `copy` from `face` -- i.e. it dereferenced the iterator. For the past-the-end iterator (`end == nb_faces` at the top level) that reads D `index_t` values one face beyond the faces buffer. `FaceIterator`'s copy constructor rebuilt the `Face` from the raw pointer too, so every copy std::sort made of the end iterator repeated the over-read. This is reachable from the public API: `dt_mesh(..., signed=true, naive=false)` -> `distance_mesh::sdt` -> `build_tree`. AddressSanitizer aborts the existing cpu-lib mesh test on it: ==25759==ERROR: AddressSanitizer: heap-buffer-overflow READ of size 4 at 0x508000000200 #0 PointMixin<3,int,StaticPoint<3,int>>::copy_(...) mesh_utils.h:124 #1 StaticPoint<3,int>::StaticPoint(AnyConstPoint<3,int> const&) #2 MeshDist<3,float,int,int>::Face::Face(int*, int) mesh.h:673 #3 MeshDist<3,float,int,int>::FaceIterator::FaceIterator(int*,int,int) #4 operator+(FaceIterator const&, int) mesh.h:743 #5 MeshDist<3,float,int,int>::build_tree<...> mesh.h:833 ... #11 ff::cpu::dt_mesh(...) distance.cpp:732 0x508000000200 is located 0 bytes after 96-byte region allocated by ff::cpu::distance_mesh::copy_faces<3,int,int> Fix, without touching the sort machinery's semantics for valid iterators: * `Face(index_t*, offset_t)` no longer loads `copy`. Every dereference of a valid iterator goes through `operator*` -> `load_()`, which still loads, so sorted output is unchanged; only the never- dereferenced end iterator stops reading. * `Face(const Face&)` snapshots `other.copy` instead of re-reading through the shared data pointer. For a loaded `other` these are the same values (that is how std::sort's `__val` temporaries already worked); for the end iterator it avoids re-triggering the read. * `FaceIterator(const FaceIterator&)` delegates to `Face`'s copy constructor rather than rebuilding from the raw pointer. * `copy` is value-initialised so an unloaded `Face` never hands out indeterminate indices when copied. * Drive-by: `Face::load()` built `clone`, loaded it, then returned `*this` -- the const `operator*` therefore returned an unloaded Face. Return `clone`. Verification: * `make -C fastfields-cpu-lib test CXX=clang++` -- all 12 suites PASS, with check counts byte-identical to the pre-fix baseline (mesh: 4622 checks / 0 failures), so this is behaviour-preserving. * ASan+UBSan build of `tests/test_distance_mesh.cpp` + `distance.cpp` (g++ -fsanitize=address,undefined): before, it aborted on the heap-buffer-overflow; after, zero heap-buffer-overflow reports and the suite runs to completion (4622 checks, 0 failures). * `nvcc -x cu` compile of fastfields-cuda-lib/distance.cpp is clean -- the touched block is inside `#ifndef __CUDACC__`, so the device build is unaffected. Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Under
__CUDACC__thehalfspecializations inutils.hfailed to compile with nvcc 12:sqrt<half>called::hsqrt, not visible at global scope in every CUDA/arch combination → compute infloatand narrow back.min<half>/max<half>compared thehalfvalues directly (a < b), ambiguous becausehalfhas multiple implicit conversions to built-in types → compare the already-computedfloatforms (af < bf).Header-only, guarded by
#ifdef __CUDACC__— no functional change on the CPU (tested) path. Surfaced by the cuda-lib-x cubuild.Closes#4. Unblocks fastfields/fastfields-cuda-lib#8.
Generated by Claude Code