Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Avoid unwanted signed/unsigned conversion check in pointer + and - #8933

Merged
merged 1 commit into from
Nov 10, 2014

Conversation

toivoh
Copy link
Contributor

@toivoh toivoh commented Nov 7, 2014

After #8420, each use of +(::Ptr,::Int) and -(::Ptr,::Int) includes a (probably unintentional) check for illegal signed/unsigned conversion: (maybe also for other Integer subtypes)

julia> code_native(+, (Ptr{Int}, Int))
    .text
Filename: pointer.jl
Source line: 65
    push    RBP
    mov RBP, RSP
    test    RSI, RSI
Source line: 65
    js  8
    add RDI, RSI
    mov RAX, RDI
    pop RBP
    ret
    movabs  RAX, 140346252604120
    mov RDI, QWORD PTR [RAX]
    movabs  RAX, 140346237622048
    mov ESI, 65
    call    RAX

I can only guess that this was not intended. Pointer arithmetic is inherently unsafe and impossible to check automatically, but it can be quite useful in tight hand-optimized loops, where the above check adds overhead and precludes further optimizations.

This patch brings back the previous, simple behavior:

julia> code_native(+, (Ptr{Int}, Int))
    .text
Filename: pointer.jl
Source line: 65
    push    RBP
    mov RBP, RSP
Source line: 65
    lea RAX, QWORD PTR [RDI + RSI]
    pop RBP
    ret

I put RFC in the title to make sure that this is actually desirable to fix, and to get feedback on the way that I have fixed it:

  • The new code assumes implicitly that a UInt has the same size as a Ptr. Afaik this is always the case in Julia, but should I rely on it?
  • A new fix will be needed if we get checked arithmetic by default.

Btw, I noticed that -(::Ptr,::Ptr) gives an unsigned result. Might also be worth fixing?

JeffBezanson added a commit that referenced this pull request Nov 10, 2014
RFC: Avoid unwanted signed/unsigned conversion check in pointer + and -
@JeffBezanson JeffBezanson merged commit a1411ca into JuliaLang:master Nov 10, 2014
@JeffBezanson
Copy link
Sponsor Member

Yes I agree -(::Ptr,::Ptr) should be Int.

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.

2 participants