Skip to content

Comments

NativeAOT/Windows/Arm64: Add TLS inline support#104282

Merged
kunalspathak merged 13 commits intodotnet:mainfrom
kunalspathak:tls-win-arm64
Jul 3, 2024
Merged

NativeAOT/Windows/Arm64: Add TLS inline support#104282
kunalspathak merged 13 commits intodotnet:mainfrom
kunalspathak:tls-win-arm64

Conversation

@kunalspathak
Copy link
Contributor

@kunalspathak kunalspathak commented Jul 2, 2024

The relocation for TLS variable is done by emitting a pair of add instructions in the following format:

add         x1,x1,tls_InlinedThreadStatics,lsl #0xC
add         x1,x1,tls_InlinedThreadStatics

The relocations are recorded as follows:

 00000030  SECREL_HIGH12A             91400021        62  tls_InlinedThreadStatics
 00000034  SECREL_LOW12L              91000021        62  tls_InlinedThreadStatics

The final disassembly looks like:

00007FF6B7AFA6E4  ldr         x0,[xpr,#0x58]  
00007FF6B7AFA6E8  adrp        x1,WKS::gc_heap::generation_table+420h (07FF6B7BFD000h)  
00007FF6B7AFA6EC  add         x1,x1,#0x2CC  
00007FF6B7AFA6F0  ldr         w1,[x1]  
00007FF6B7AFA6F4  ldr         x0,[x0,x1 lsl #3]  
00007FF6B7AFA6F8  add         x1,x1,#0,lsl #0xC  
00007FF6B7AFA6FC  add         x1,x1,#4

Contributes to #79521

@kunalspathak
working with 1 change remaining
12fa9bc 
hack to test

some fixes

almost trhere

fix assert

Some comments and revert sample Program

some renaming

Revert changes for IF_DI_1E

format IF_DI_2A code:

Format emitIns_Add_Add_Tls_Reloc

Fixed the code in IF_DI_2A
@kunalspathak
revert changes in reproNative.vcxproj
a1f1738
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 2, 2024
@kunalspathak kunalspathak mentioned this pull request Jul 2, 2024
Closed
@kunalspathak kunalspathak requested a review from jkotas July 2, 2024 04:12
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Jul 2, 2024

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

@kunalspathak
Copy link
Contributor Author

kunalspathak commented Jul 2, 2024

@dotnet/jit-contrib

kunalspathak
kunalspathak commented Jul 2, 2024
View reviewed changes
@kunalspathak kunalspathak changed the title Windows/Arm64: Adds TLS inline support NativeAOT/Windows/Arm64: Add TLS inline support Jul 2, 2024
jkotas
jkotas reviewed Jul 2, 2024
View reviewed changes
@kunalspathak
Copy link
Contributor Author

kunalspathak commented Jul 2, 2024

I forgot to update the guid. Will do it once i see some superpmi diff runs for JIT arm64, because I changed a code in lowering.

jakobbotsch
jakobbotsch reviewed Jul 2, 2024
View reviewed changes
@kunalspathak kunalspathak requested a review from VSadov July 2, 2024 15:37
jakobbotsch
jakobbotsch reviewed Jul 2, 2024
View reviewed changes
jakobbotsch
jakobbotsch reviewed Jul 2, 2024
View reviewed changes
jakobbotsch
jakobbotsch approved these changes Jul 2, 2024
View reviewed changes
Copy link
Member

@jakobbotsch jakobbotsch left a comment

Choose a reason for hiding this comment

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

JIT side LGTM with some nits.

@kunalspathak
Copy link
Contributor Author

kunalspathak commented Jul 2, 2024

@jkotas @VSadov - this is ready again. Let me know if you have more feedback.

jkotas
jkotas reviewed Jul 2, 2024
View reviewed changes
VSadov
VSadov reviewed Jul 2, 2024
View reviewed changes
src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
const ushort IMAGE_REL_ARM64_PAGEBASE_REL21 = 4;
const ushort IMAGE_REL_ARM64_PAGEOFFSET_12A = 6;
const ushort IMAGE_REL_ARM64_SECREL_HIGH12A = 0xA;
const ushort IMAGE_REL_ARM64_SECREL_LOW12L = 0xB;
Copy link
Member

@VSadov VSadov Jul 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Nit: some constants here match values from RelocType and some match the actual platform-specific values (for the ease of disassembling?).

If that is a case, then maybe always use platform-specific values?
I.E. IMAGE_REL_ARM64_TLSDESC_CALL would match R_AARCH64_TLSDESC_CALL, which is 0x239

Or the other way - just use RelocType values for all?

Copy link
Member

@VSadov VSadov Jul 2, 2024

Choose a reason for hiding this comment

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

Does not need to be addressed in this change since the inconsistency is already there.

Copy link
Contributor Author

@kunalspathak kunalspathak Jul 2, 2024

Choose a reason for hiding this comment

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

yeah, probably will not include in this PR.

@kunalspathak @jkotas
Update src/coreclr/tools/Common/Compiler/DependencyAnalysis/Relocatio…
565f2d7 
…n.cs

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@build-analysis build-analysis bot mentioned this pull request Jul 3, 2024
Closed
VSadov
VSadov approved these changes Jul 3, 2024
View reviewed changes
Copy link
Member

@VSadov VSadov left a comment

Choose a reason for hiding this comment

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

LGTM!! Thanks!!

@kunalspathak kunalspathak merged commit f92499d into dotnet:main Jul 3, 2024
@kunalspathak kunalspathak deleted the tls-win-arm64 branch July 3, 2024 05:03
This was referenced Jul 4, 2024
Closed
Merged
@jkotas jkotas mentioned this pull request Jul 6, 2024
Closed
@JulieLeeMSFT JulieLeeMSFT mentioned this pull request Jul 9, 2024
Closed
@kunalspathak kunalspathak mentioned this pull request Jul 26, 2024
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jkotas jkotas jkotas left review comments

@jakobbotsch jakobbotsch jakobbotsch approved these changes

@VSadov VSadov VSadov approved these changes

@MichalStrehovsky MichalStrehovsky Awaiting requested review from MichalStrehovsky MichalStrehovsky is a code owner

Assignees

@kunalspathak kunalspathak

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kunalspathak @jkotas @jakobbotsch @VSadov