Skip to content

Conversation

michel2323
Copy link
Member

@michel2323 michel2323 commented Oct 8, 2025

In Julia >=1.12, the SPIRV translator encounters Int128s when running the oneAPI.jl tests. This PR truncates Int128 to Int64 and prevents the translator from crashing. Additionally, it provides the user with clear information on the truncation.

See JuliaGPU/oneAPI.jl#529 and #101

@michel2323 michel2323 requested review from maleadt and vchuravy October 8, 2025 19:29
Copy link

codecov bot commented Oct 8, 2025

Codecov Report

❌ Patch coverage is 21.81818% with 86 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.67%. Comparing base (6c7d659) to head (5885f2a).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/spirv.jl 21.81% 86 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #728      +/-   ##
==========================================
- Coverage   75.26%   73.67%   -1.60%     
==========================================
  Files          24       24              
  Lines        3635     3745     +110     
==========================================
+ Hits         2736     2759      +23     
- Misses        899      986      +87     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vchuravy
Copy link
Member

vchuravy commented Oct 8, 2025

Oof that's a tough one. I feel like we should actually do the soft implementation instead of truncating to the lowbit

@michel2323
Copy link
Member Author

michel2323 commented Oct 8, 2025 via email

@vchuravy
Copy link
Member

vchuravy commented Oct 9, 2025

With "soft math" I mean doing the operations in software instead of using hardware operations. So do the lowering as you are here converting int128 into {i64, i64} and then instead of silently truncating it, perform a software i128 addition / multiplication.

As an example GCC https://gcc.gnu.org/onlinedocs/gccint/Integer-library-routines.html and LLVM https://github.com/llvm/llvm-project/tree/main/compiler-rt/lib/builtins

https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/builtins/multi3.c

(Note that this is similar to what we could/should do for platforms that don't have Float64)

@michel2323
Copy link
Member Author

michel2323 commented Oct 9, 2025

Phew. That's what I meant with teaching the translator Int128. That's quite a task. Let's say what about this as an interim fix? ;-)

I somehow wonder why this Int128 is even emitted. It happens only in one test. I looked around a bit, but it seems that LLVM doesn't provide any built-in mechanism for Int128 -> Int64, Int64. Don't most backends have to implement it, since most hardware has no 128bit registers? Should I ask the AMDGPU folks? Can't we blame the frontend?

Float64/128 emulation through Float32/Float64 is really a can of worms.

@maleadt
Copy link
Member

maleadt commented Oct 9, 2025

Simply truncating seems bad indeed. Did you consider adding a quirk instead to override the Julia code that widens in the first place?

@michel2323
Copy link
Member Author

No. Where do I add a quirk? I thought this was what you meant by quirk.

@maleadt
Copy link
Member

maleadt commented Oct 9, 2025

@vchuravy
Copy link
Member

vchuravy commented Oct 9, 2025

Can't we blame the frontend?

We are the frontend :/

@michel2323
Copy link
Member Author

Thanks both of you! It was indeed an easy fix. This widemul was the issue https://github.com/JuliaLang/julia/blob/92af0d8cdf28c3e58c95469366ee64c4d06e5cd6/base/range.jl#L962

It only triggered with --check-bounds=on so I used the solution of Metal.jl: https://github.com/JuliaGPU/Metal.jl/blob/18d5d9588d6fa0b387ccea065e712fdf5ccb5b57/src/device/quirks.jl#L87-L94

@michel2323 michel2323 closed this Oct 9, 2025
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.

3 participants