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.

Sign up for GitHub

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

Jump to bottom

[RISC-V] Enable TLS field access optimization #89561

Merged
merged 1 commit into from
Jul 28, 2023
Merged

[RISC-V] Enable TLS field access optimization #89561

merged 1 commit into from
Jul 28, 2023

Conversation

tomeksowi
Copy link
Contributor

@tomeksowi tomeksowi commented Jul 27, 2023 •
edited
Loading

Re-enable inline TLS field access optimization which was disabled in #88584 as a temporary measure to fix failing tests. The implementation is analogous to the PR #88819 for loongarch64.

Part of #84834
cc @wscho77 @HJLeee @JongHeonChoi @t-mustafin @alpencolt @gbalykov @clamp03

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 27, 2023
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jul 27, 2023
@ghost
Copy link

ghost commented Jul 27, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

Re-enable TLS field access optimization which was disabled in #88584 as a temporary measure to fix failing tests. The implementation is analogous to the PR #88819 for loongarch64.

Part of #84834
cc @wscho77 @HJLeee @JongHeonChoi @t-mustafin @alpencolt @gbalykov @clamp03

Author: tomeksowi
Assignees: -
Labels:

area-CodeGen-coreclr, community-contribution
Milestone: -

@tomeksowi
Copy link
Contributor Author

@dotnet-policy-service agree company="Samsung"

@clamp03 clamp03 added the arch-riscv Related to the RISC-V architecture label Jul 28, 2023
@clamp03
Copy link
Member

clamp03 commented Jul 28, 2023

@jakobbotsch @jkotas Could you please review this PR?
@shushanhf Changed near to TARGET_LOONGARCH64, maybe you would better to review before merge it.

shushanhf
shushanhf approved these changes Jul 28, 2023
View reviewed changes
Copy link
Contributor

@shushanhf shushanhf left a comment

Choose a reason for hiding this comment

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

LGTM

src/coreclr/jit/helperexpansion.cpp
//
// Code sequence to access thread local variable on linux/riscv64:
//
// mov targetReg, $tp, 0
Copy link
Contributor

Choose a reason for hiding this comment

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

mov targetReg, $tp is ok ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's what gcc/clang emits on RISC-V to access a thread local var.

jkotas
jkotas approved these changes Jul 28, 2023
View reviewed changes
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks

@jkotas jkotas merged commit f11f905 into dotnet:main Jul 28, 2023
125 checks passed
@HJLeee
Copy link
Contributor

HJLeee commented Jul 28, 2023

@tomeksowi Congratulations for the first landing. :)

@ghost ghost locked as resolved and limited conversation to collaborators Aug 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@shushanhf shushanhf shushanhf approved these changes

@jkotas jkotas jkotas approved these changes

@clamp03 clamp03 clamp03 approved these changes

@alpencolt alpencolt alpencolt approved these changes

Assignees

@tomeksowi tomeksowi

Labels
arch-riscv Related to the RISC-V architecture area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tomeksowi @clamp03 @HJLeee @alpencolt @jkotas @shushanhf