Skip to content

Comments

feat: Implement issorted for AbstractGPUArray without scalar indexing.#678

Merged
maleadt merged 2 commits intoJuliaGPU:masterfrom
shreyas-omkar:master
Jan 22, 2026
Merged

feat: Implement issorted for AbstractGPUArray without scalar indexing.#678
maleadt merged 2 commits intoJuliaGPU:masterfrom
shreyas-omkar:master

Conversation

@shreyas-omkar
Copy link
Contributor

Fix for #635

Base.issorted on AbstractGPUArray currently falls back to scalar iteration, which triggers scalar indexing errors and prevents correct usage on GPU-backed arrays.

This PR introduces a GPU-native implementation of issorted using a kernel-based approach, ensuring the operation executes entirely on the device.

Tests

(GPUArrays with base as test_args)

image

(GPUArrays)
Screenshot 2026-01-15 181343

@shreyas-omkar
Copy link
Contributor Author

@kshyatt please review.

@kshyatt
Copy link
Member

kshyatt commented Jan 16, 2026

Thanks for getting to this so quickly! I'm at a conference today so may not get to it immediately but perhaps @maleadt has spare time? If not I'll look on Monday.

@shreyas-omkar
Copy link
Contributor Author

shreyas-omkar commented Jan 16, 2026

@maleadt I have removed the unnecessary using KernelAbstractions import as suggested

I initially tried using Base.Order.lt(ord, a, b), but this results in invalid GPU IR.
To avoid this, the current implementation resolves order and rev on the host and passes plain lt / by functions into the kernel, which keeps the device code simple and GPU-safe.

Please let me know if I'm missing something.

@maleadt
Copy link
Member

maleadt commented Jan 19, 2026

Seems to generate code fine? What issue exact were you running into?

julia> code_llvm(Base.lt, Tuple{Base.Order.ForwardOrdering, Int, Int})
; Function Signature: lt(Base.Order.ForwardOrdering, Int64, Int64)
;  @ ordering.jl:118 within `lt`
define i8 @julia_lt_3166(i64 signext %"a::Int64", i64 signext %"b::Int64") #0 {
top:
; ┌ @ operators.jl:431 within `isless`
; │┌ @ int.jl:83 within `<`
    %0 = icmp slt i64 %"a::Int64", %"b::Int64"
    %1 = zext i1 %0 to i8
    ret i8 %1
; └└
}

julia> code_llvm(Base.lt, Tuple{Base.Order.ReverseOrdering{Base.Order.ForwardOrdering}, Int, Int})
; Function Signature: lt(Base.Order.ReverseOrdering{Base.Order.ForwardOrdering}, Int64, Int64)
;  @ ordering.jl:119 within `lt`
define i8 @julia_lt_3169(i64 signext %"a::Int64", i64 signext %"b::Int64") #0 {
top:
;  @ ordering.jl:119 within `lt` @ ordering.jl:118
; ┌ @ operators.jl:431 within `isless`
; │┌ @ int.jl:83 within `<`
    %0 = icmp slt i64 %"b::Int64", %"a::Int64"
    %1 = zext i1 %0 to i8
    ret i8 %1
; └└
}

@github-actions
Copy link
Contributor

github-actions bot commented Jan 19, 2026

Your PR no longer requires formatting changes. Thank you for your contribution!

@shreyas-omkar
Copy link
Contributor Author

Hi @maleadt . I have made the changes that you've suggested.

Copy link
Member

@maleadt maleadt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

@maleadt maleadt merged commit efdc7cf into JuliaGPU:master Jan 22, 2026
19 checks passed
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