Repository navigation
Cadence: keep int64 precision in div trunc mode and HiFi fmod - #24349
JakeStevens wants to merge 2 commits into
Conversation
The HiFi and Fusion G3 div_out_mode (and G3 div_scalar_mode_out) kernels called std::trunc on the integer quotient, which rounds int64 values above 2^53 through double. Integer division already truncates, so trunc now only runs for floating-point types. This mirrors the portable fix for pytorch#24169. HiFi fmod.Tensor_out and fmod.Scalar_out forced every integer compute type to Double. Integers are now computed with %, returning 0 for a divisor of -1 like ATen. fmod.Scalar_out checks for a zero divisor after casting the scalar to the compute dtype, so 256 for an int8 tensor fails instead of reaching a % 0. HiFi fmod.Tensor_out also had an inverted NNLib fast-path condition: Float inputs always took the std::fmod fallback, and every other dtype was sent to xa_nn_elm_fmod_f32xf32_f32, which read integer data as float. The fast path is removed rather than fixed, because the NNLib kernel computes a - trunc(a / b) * b in float32, which is not exact fmod (1.0f and 0.1f give 0) and would change today's Float results. This change was authored with AI assistance (Claude Code).
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24349
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ❌ 1 New FailureAs of commit 1ea7b32 with merge base cb3f24e ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
|
Probably worth importing internally for verification. |
|
@mcremon-meta has imported this pull request. If you are a Meta employee, you can view this in D129098757. |
This PR needs a
|
Summary
The Cadence HiFi and Fusion G3 operators are forks of the portable kernels and carry the int64 precision bug from #24169, which #24170 fixes in portable.
HiFi and G3
div_out_mode(and G3div_scalar_mode_out) calledstd::truncon the integer quotient, rounding int64 values above 2^53 throughdouble. Integer division already truncates, sotruncnow only runs for floating-point types.HiFi
fmod.Tensor_outandfmod.Scalar_outforced every integer compute type toDouble. Integers are now computed with%, returning 0 for a divisor of -1 like ATen.fmod.Scalar_outchecks for a zero divisor after casting the scalar to the compute dtype, so 256 for an int8 tensor fails instead of reachinga % 0; empty inputs skip the check like ATen. The localfmod_valuehelper can switch toutils::fmod_overrideonce #24170 lands.HiFi
fmod.Tensor_outalso had an inverted NNLib fast-path condition: Float inputs always took thestd::fmodfallback, and every other dtype with rank > 0 was sent toxa_nn_elm_fmod_f32xf32_f32, which read integer data as float. The fast path is removed rather than fixed: the NNLib kernel computesa - trunc(a / b) * bin float32, which is not exact fmod (1.0f and 0.1f give 0 instead of ~0.1), so enabling it would change today's Float results. Float behavior is unchanged by this PR.Test plan
I could not build or run the Xtensa targets locally. All three changed files pass a host
clang++ -std=c++17 -fsyntax-only -Wallcheck against stub NNLib headers, which instantiates every dtype in the switches. The integer fmod and divisor logic matches the portable kernels tested in #24170 and the portable remainder fix. There is no existing HiFi fmod test target to extend; relying on Cadence CI for the build.This PR was authored with AI assistance (Claude Code), with an additional review pass by Codex.