Skip to content

Detect Int64 SUM overflow on the CUDA aggregate path instead of wrapping #46

Description

@vyncint

Todaycrates/oxidelake-device/src/cuda/ops.rs:10-11: "Divergence from the CPU reference, by design: device SUM over Int64 wraps on overflow where the CPU path reports an error." The kernel uses atomicAdd((unsigned long long*)&sums[slot], …) (kernels/cuda/aggregation.cu:125); the CPU path errors with "SUM overflowed Int64" (crates/oxidelake-device/src/cpu/kernels.rs:439-441). Conformance has no overflow case (grep -n overflow conformance.rs → 0).

Why it is worth fixing — a documented wrong answer is still a wrong answer, and DataFusion's stock aggregate errors here. Under the equality-with-stock-DataFusion standard the conformance suite enforces, this is the one divergence left in by design.

Fix — accumulate an overflow flag on device (check __builtin_add_overflow-style before the atomic, or keep a per-slot high word and detect carry) and return EngineError::Execution("SUM overflowed Int64") on the host; add an overflow row (i64::MAX - 1 twice in one group) to conformance.

Done when — CPU, CUDA (GPU job) and stock DataFusion all error on the fixture; the "by design" comment is removed.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions