Skip to content

catch/math: fix std::min type deduction where size_t != uint64_t - #2

Open
Noerr wants to merge 1 commit into
CHIP-SPV:develop-chipstarfrom
Noerr:fix-macos-mathstest-build
Open

Noerr wants to merge 1 commit into
CHIP-SPV:develop-chipstarfrom
Noerr:fix-macos-mathstest-build

Conversation

@Noerr

@Noerr Noerr commented Sep 3, 2026

Copy link
Copy Markdown

std::min deduces a single _Tp for both parameters, so both arguments must have the same
type. Ten call sites under catch/unit/math/ mix a size_t-typed expression — the result
of GetMaxAllowedDeviceMemoryUsage() / sizeof(...) — with a uint64_t iteration bound:

uint64_t stop = std::numeric_limits<uint16_t>::max() + 1ul;
const auto max_batch_size =
    std::min(GetMaxAllowedDeviceMemoryUsage() / (sizeof(Float16) + sizeof(T)), stop);

That fails to deduce on any target where size_t and uint64_t are spelled differently:

error: no matching function for call to 'min'
note: candidate template ignored: deduced conflicting types for parameter '_Tp'
      ('unsigned long' vs. 'uint64_t' (aka 'unsigned long long'))
  • Darwin (any arch): size_t is unsigned long, uint64_t is unsigned long long
  • ILP32 targets such as i386 / armv7: 4-byte size_t vs 8-byte uint64_t

It compiles on Linux LP64 and Windows LLP64 only because the two spellings coincide there,
so this is latent rather than absent on those hosts.

The fix uses the idiom already present a few lines below each of these sites — for example
unary_common.hh:61 — an explicit std::min<uint64_t>. Widening to uint64_t rather than
narrowing to size_t is the correct direction: on LP64 the two are both 8 bytes so nothing
changes, while on ILP32 narrowing would truncate.

No behavioural change: on LP64 the types are the same width and the values are unaffected.
This only lets deduction succeed.

Files touched: unary_common.hh, binary_common.hh, ternary_common.hh,
quaternary_common.hh, casting_common.hh, pow_common.hh, special_common.hh.

Scope

This does not by itself make MathsTest build on macOS — there are further, unrelated
failures in that target. I opened a separate issue covering those: CHIP-SPV/chipStar/issues/1583

std::min takes both arguments as a single deduced const _Tp&, so it needs one
type. Ten call sites mixed a size_t-typed expression -- the result of
GetMaxAllowedDeviceMemoryUsage() / sizeof(...) -- with a uint64_t iteration
bound, which cannot deduce on any target where those two are spelled
differently.

That is Darwin (size_t is unsigned long, uint64_t is unsigned long long, both
8 bytes) and any ILP32 target (4-byte size_t, 8-byte uint64_t). It compiles on
Linux LP64 and Windows LLP64 only because the spellings happen to coincide
there, so the bug is latent rather than absent on those hosts.

The fix is the idiom already used by the adjacent batch-size clamps a few lines
below each site, e.g. unary_common.hh:61 -- an explicit std::min<uint64_t>.
Widening to uint64_t rather than narrowing to size_t matters on ILP32, where
narrowing would truncate.

No behavioural change on LP64: the two types are the same width and the values
are unaffected; this only lets deduction succeed.
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.

1 participant