Skip to content

Conversation

@pchintalapudi
Copy link
Member

PTLS load instructions can interfere with optimizing allocations due to being unhoistable and inseparable from their corresponding gc_alloc_obj call. By moving their generation to late-gc-lowering, we can optimize allocation function calls directly without worry about the PTLS load.

@vchuravy vchuravy added the compiler:codegen Generation of LLVM IR and native code label Oct 9, 2021
Copy link
Member

@vchuravy vchuravy left a comment

Choose a reason for hiding this comment

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

@pchintalapudi thanks for opening this PR.

IIRC you had a couple of testcases that we were looking at? Could you add them to test/llvmpasses?

@vchuravy
Copy link
Member

@nanosoldier runtests(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

@vchuravy vchuravy requested a review from vtjnash October 10, 2021 20:07
@vchuravy
Copy link
Member

@vtjnash it is my understanding that currently we need to always redo the ptls load from the current_task. @pchintalapudi is looking at enabling hoisting of allocations from loops and with the ptls load the allocation call is no longer invariant.

Do you see any reason for why emitting the load earlier might be benefitial? (Outside starting to add invariant range annotations.)

@vtjnash
Copy link
Member

vtjnash commented Oct 12, 2021

I'd suggest passing pptls there, since the GEP computation adds unnecessary cost, but otherwise SGTM

@vtjnash
Copy link
Member

vtjnash commented Oct 12, 2021

Actually, I guess later optimization passes are likely to fold that away regardless

@pchintalapudi pchintalapudi force-pushed the pc/ptls-codegen branch 2 times, most recently from d3728ce to e5160eb Compare October 13, 2021 06:12
@vchuravy
Copy link
Member

@pchintalapudi the last thing you will need to fix is the llvmpasses failure https://buildkite.com/julialang/julia-master/builds/4661#78e454c5-f38e-4605-b5ed-0346286be2f0

@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Oct 26, 2021
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Oct 28, 2021
@DilumAluthge
Copy link
Member

DilumAluthge commented Oct 28, 2021

@pchintalapudi Can you rebase this PR on the latest master? This should fix the Downloads tests.

@vchuravy vchuravy merged commit f8c918b into JuliaLang:master Nov 10, 2021
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
…2572)

* Move PTLS load emission from codegen to late-gc-lowering

* Address PR comments

* Fix tests and move TBAA node generation to codegen_shared

* Add check for null metadata node

Co-authored-by: Prem Chintalapudi <[email protected]>
@pchintalapudi pchintalapudi deleted the pc/ptls-codegen branch March 6, 2022 21:57
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
…2572)

* Move PTLS load emission from codegen to late-gc-lowering

* Address PR comments

* Fix tests and move TBAA node generation to codegen_shared

* Add check for null metadata node

Co-authored-by: Prem Chintalapudi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compiler:codegen Generation of LLVM IR and native code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants