expand memoryrefnew capabilities #58768
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nsajko
reviewed
Jun 18, 2025
nsajko
reviewed
Jun 18, 2025
|
Compiler bootstrap fails. Analyzegc says there are three errors: analyzegc errors
|
|
analyzegc issue was pretty simple. |
oscardssmith
force-pushed
the
os/more-memoryrefnew
branch
from
f51b1ea to
19e1d93
Compare
oscardssmith
commented
Jun 19, 2025
| @@ -720,7 +720,9 @@ function iscall_with_boundscheck(@nospecialize(stmt), sv::PostOptAnalysisState) | |||
| f === nothing && return false | |||
| if f === getfield | |||
| nargs = 4 | |||
| elseif f === memoryrefnew || f === memoryrefget || f === memoryref_isassigned | |||
| elseif f === memoryrefnew | |||
There was a problem hiding this comment.
This one is a fun bugfix...
|
Now there's only one failing test |
|
yeah. that said, before I fix that, it's probably time to actually add the different simplification that was the main point of this pr |
|
Codegen added! (thanks @gbaraldi), code looks a lot better now. for julia> ms = Memory{Int}(undef, 5)
julia> @code_native debuginfo=:none m2[3]we now get .file "getindex"
.section .ltext,"axl",@progbits
.globl julia_getindex_1350 # -- Begin function julia_getindex_1350
.p2align 4
.type julia_getindex_1350,@function
julia_getindex_1350: # @julia_getindex_1350
; Function Signature: getindex(Memory{Int64}, Int64)
# %bb.0: # %top
#DEBUG_VALUE: getindex:A <- [$rdi+0]
#DEBUG_VALUE: getindex:i <- $rsi
lea rax, [rsi - 1]
cmp rax, qword ptr [rdi]
jae .LBB0_2
# %bb.1: # %L14
mov rax, qword ptr [rdi + 8]
mov rax, qword ptr [rax + 8*rsi - 8]
ret
.LBB0_2: # %L10
push rbp
mov rbp, rsp
sub rsp, 16
mov qword ptr [rbp - 8], rsi
movabs rax, offset j_throw_boundserror_1352
lea rsi, [rbp - 8]
call rax
.Lfunc_end0:while before it was .text
.file "getindex"
.section .ltext,"axl",@progbits
.globl julia_getindex_1455 # -- Begin function julia_getindex_1455
.p2align 4, 0x90
.type julia_getindex_1455,@function
julia_getindex_1455: # @julia_getindex_1455
; Function Signature: getindex(Memory{Int64}, Int64)
# %bb.0: # %top
#DEBUG_VALUE: getindex:A <- [$rdi+0]
#DEBUG_VALUE: getindex:i <- $rsi
push rbp
mov rbp, rsp
push r15
push r14
push r12
push rbx
mov rax, qword ptr [rdi]
mov r15, qword ptr [rdi + 8]
lea rdx, [rax + rsi - 1]
lea rcx, [rax + rax]
cmp rdx, rcx
jae .LBB0_3
# %bb.1: # %top
lea rcx, [8*rsi - 8]
shl rax, 3
cmp rcx, rax
jae .LBB0_3
# %bb.2: # %idxend
mov rax, qword ptr [r15 + 8*rsi - 8]
pop rbx
pop r12
pop r14
pop r15
pop rbp
ret
.LBB0_3: # %oob
#APP
mov rax, qword ptr fs:[0]
#NO_APP
mov rax, qword ptr [rax - 8]
movabs rbx, 138975374889648
mov r14, rsi
movabs r8, offset ijl_gc_small_alloc
mov esi, 408
mov edx, 32
mov r12, rdi
mov rcx, rbx
mov rax, qword ptr [rax + 16]
mov rdi, rax
call r8
movabs rcx, offset ijl_bounds_error_int
mov qword ptr [rax - 8], rbx
mov qword ptr [rax], r15
mov qword ptr [rax + 8], r12
mov rdi, rax
mov rsi, r14
call rcx
.Lfunc_end0: |
|
@nanosoldier |
|
@nanosoldier |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. |
oscardssmith
force-pushed
the
os/more-memoryrefnew
branch
from
d4b610d to
0b6f8d4
Compare
| const jl_datatype_layout_t *layout = ((jl_datatype_t*)jl_typetagof(m->mem))->layout; | ||
| char *data = (char*)m->ptr_or_offset; | ||
| char *data; | ||
| if(jl_is_genericmemory(args[0])) { |
There was a problem hiding this comment.
Suggested change
| if(jl_is_genericmemory(args[0])) { | |
| if (jl_is_genericmemory(args[0])) { |
|
nanosoldier regressions do not reproduce locally. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal here is 2-fold. Firstly, this should let us simplify the boundscheck (not yet implimented), but this also should reduce Julia IR side a bit.