Skip to content

add specialized codegen for jl_get_current_task#32812

Merged
JeffBezanson merged 1 commit intomasterfrom
jb/ctcg
Mar 10, 2020
Merged

add specialized codegen for jl_get_current_task#32812
JeffBezanson merged 1 commit intomasterfrom
jb/ctcg

Conversation

@JeffBezanson
Copy link
Copy Markdown
Member

@JeffBezanson JeffBezanson commented Aug 6, 2019

The code_llvm for this seems to be nastier than I'd expect:

julia> @code_llvm current_task()

;  @ task.jl:95 within `current_task'
define nonnull %jl_value_t addrspace(10)* @jsys1_current_task_15548(%jl_value_t addrspace(10)*, %jl_value_t addrspace(10)**, i32) #0 {
top:
  %3 = alloca %jl_value_t addrspace(10)**, align 8
  store volatile %jl_value_t addrspace(10)** %1, %jl_value_t addrspace(10)*** %3, align 8
  %thread_ptr = call i8* asm "movq %fs:0, $0", "=r"()
  %ptls_i8 = getelementptr i8, i8* %thread_ptr, i64 -15712
  %ptls = bitcast i8* %ptls_i8 to %jl_value_t***
  %4 = getelementptr %jl_value_t**, %jl_value_t*** %ptls, i64 826
  %5 = bitcast %jl_value_t*** %4 to %jl_value_t addrspace(10)**
  %6 = load %jl_value_t addrspace(10)*, %jl_value_t addrspace(10)** %5, align 8
  ret %jl_value_t addrspace(10)* %6
}

Why is this using a less efficient calling convention, and what's with the volatile store?

@JeffBezanson JeffBezanson added the compiler:codegen Generation of LLVM IR and native code label Aug 6, 2019
@yuyichao
Copy link
Copy Markdown
Contributor

yuyichao commented Aug 6, 2019

The signature was #11306 (IIRC the logic looks for non jl_value_t* types and isn't triggered on empty argument) The logic there was to reduce allocation without considering jlcall frame allocation.

I believe the volatile store is there to help the debugger work by making sure the jlcall frame address is always available.

@JeffBezanson JeffBezanson added multithreading Base.Threads and related functionality performance Must go faster labels Aug 10, 2019
@chethega chethega mentioned this pull request Feb 26, 2020
Closed
@chethega
Copy link
Copy Markdown
Contributor

chethega commented Feb 29, 2020

Bump. FWIW, my patch in #34852 generates

julia> @code_llvm current_task()

;  @ task.jl:120 within `current_task'
define nonnull %jl_value_t addrspace(10)* @julia_current_task_16641() {
top:
  %thread_ptr = call i8* asm "movq %fs:0, $0", "=r"()
  %ptls_i8 = getelementptr i8, i8* %thread_ptr, i64 -15712
  %ptls = bitcast i8* %ptls_i8 to %jl_value_t***
  %0 = getelementptr %jl_value_t**, %jl_value_t*** %ptls, i64 826
  %1 = bitcast %jl_value_t*** %0 to %jl_value_t addrspace(10)**
  %2 = load %jl_value_t addrspace(10)*, %jl_value_t addrspace(10)** %1, align 8
  ret %jl_value_t addrspace(10)* %2
}

I have no idea why, since your patch looks almost the same as mine, maybe something else got rid of the store volatile?

@JeffBezanson
Copy link
Copy Markdown
Member Author

JeffBezanson commented Mar 4, 2020

Rebasing on master fixes it, so something else must have changed it.

@JeffBezanson JeffBezanson requested a review from vtjnash March 9, 2020 21:02
@JeffBezanson JeffBezanson merged commit 6c5fb0e into master Mar 10, 2020
@JeffBezanson JeffBezanson deleted the jb/ctcg branch March 10, 2020 19:28
@KDr2 KDr2 mentioned this pull request Apr 8, 2020
Closed
ravibitsgoa pushed a commit to ravibitsgoa/julia that referenced this pull request Apr 9, 2020
@JeffBezanson @ravibitsgoa
add specialized codegen for jl_get_current_task (JuliaLang#32812)
15d008a
KristofferC pushed a commit that referenced this pull request Apr 11, 2020
@JeffBezanson @KristofferC
add specialized codegen for jl_get_current_task (#32812)
9563961
@fingolfin fingolfin mentioned this pull request Nov 11, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vtjnash vtjnash vtjnash approved these changes

Assignees

No one assigned

Labels

compiler:codegen Generation of LLVM IR and native code multithreading Base.Threads and related functionality performance Must go faster

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@JeffBezanson @yuyichao @chethega @vtjnash