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

[NativeAOT] RhpByRefAssignRef is missing a barrier for Linux ARM32 and ARM64 #639

Closed
AntonLapounov opened this issue Feb 5, 2021 · 1 comment · Fixed by dotnet/runtime#72919
Closed

[NativeAOT] RhpByRefAssignRef is missing a barrier for Linux ARM32 and ARM64 #639

AntonLapounov opened this issue Feb 5, 2021 · 1 comment · Fixed by dotnet/runtime#72919
Labels
area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation

Comments

@AntonLapounov
Copy link
Member

There should be a dmb/stlr instruction in Linux ARM32 and ARM64 versions of RhpByRefAssignRef assembler helper:

runtimelab/src/coreclr/nativeaot/Runtime/arm/WriteBarriers.S

Lines 325 to 327 in c9c0100

LEAF_ENTRY RhpByRefAssignRef, _TEXT
ldr r2, [r1]
str r2, [r0]

runtimelab/src/coreclr/nativeaot/Runtime/arm64/WriteBarriers.S

Lines 363 to 365 in c9c0100

LEAF_ENTRY RhpByRefAssignRefArm64, _TEXT
ldr x15, [x13]
str x15, [x14]

The code was added in these commits:
dotnet/corert@39c6ceb#diff-5f1553cb1c44e19848424918ad7a351a00fce13fd08216e3d866cc41f1e0bf0fR360
dotnet/corert@e08c6b8#diff-99ef768b3a97ff13d34b55ad20908bb94982d34ff51f8b2a91efbd4dc010e6c0R289
@AntonLapounov AntonLapounov mentioned this issue Feb 5, 2021
Merged
@MichalStrehovsky MichalStrehovsky added the area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation label Feb 11, 2021
@MichalStrehovsky
Copy link
Member

Cc @RalfKornmannEnvision since I assume this might lead to stress bugs and things like this are always good to know about.

@MichalStrehovsky MichalStrehovsky mentioned this issue Jul 27, 2022
Merged
runtimelab-bot pushed a commit that referenced this issue Jul 5, 2023
@jakobbotsch
JIT: Propagate multi-reg-index for upper-vector-restore ref positions…
d52b303 
… (#88380)

`LinearScan::resolveRegisters` will write the register back to the IR
node for an upper-vector-restore RP, so without this propagation we
would overwrite the register assignment for an unrelated field.

For example, for IR like
```scala
N001 (  9,  6) [000090] m------N---                   t90 =    LCL_VAR   struct<JIT.HardwareIntrinsics.Arm._AdvSimd.SimpleTernaryOpTest__AbsoluteDifferenceWideningUpperAndAdd_Vector128_UInt32+TestStruct, 48>(P) V00 loc0
                                                            ▌    simd16 field V00._fld1 (fldOffset=0x0) -> V14 tmp10         (last use)
                                                            ▌    simd16 field V00._fld2 (fldOffset=0x10) -> V15 tmp11         (last use)
                                                            ▌    simd16 field V00._fld3 (fldOffset=0x20) -> V16 tmp12         (last use) $540
                                                            ┌──▌  t90    struct
N002 ( 10,  7) [000091] -----------                         ▌  RETURN    struct $VN.Void
```
we could build ref positions such as

```
[000091] 449.#639 U14  UVRs   UVRes    NA   │    │    │    │    │    │    │    │    │    │    │    │    │V16a│    │    │    │
         449.#640 d0   Fixd   Keep     d0   │    │    │    │    │    │    │    │    │    │    │    │    │V16a│    │    │    │
         449.#641 V14  Use *  ReLod    d0   │    │    │    │    │    │    │    │    │    │    │V14a│    │V16a│    │    │    │
                              Keep     d0   │    │    │    │    │    │    │    │    │    │    │V14i│    │V16a│    │    │    │
         449.#642 U15  UVRs   UVRes    NA   │    │    │    │    │    │    │    │    │    │    │    │    │V16a│    │    │    │
         449.#643 d1   Fixd   Keep     d1   │    │    │    │    │    │    │    │    │    │    │    │    │V16a│    │    │    │
         449.#644 V15  Use *  ReLod    d1   │    │    │    │    │    │    │    │    │    │    │    │V15a│V16a│    │    │    │
                              Keep     d1   │    │    │    │    │    │    │    │    │    │    │    │V15i│V16a│    │    │    │
         449.#645 d2   Fixd   Keep     d2   │    │    │    │    │    │    │    │    │    │    │    │    │V16a│    │    │    │
         449.#646 V16  Use *  Keep     d2   │    │    │    │    │    │    │    │    │    │    │    │    │V16i│    │    │    │
```

When writing back register assignments the upper-vector-restore at #642
ended up overwriting the assignment for the first field on [000090],
resulting in

```scala
N447 (  9,  6) [000090] m------N--z                   t90 =    LCL_VAR   struct<JIT.HardwareIntrinsics.Arm._AdvSimd.SimpleTernaryOpTest__AbsoluteDifferenceWideningUpperAndAdd_Vector128_UInt32+TestStruct, 48>(P) V00 loc0          NA
                                                            ▌    simd16 field V00._fld1 (fldOffset=0x0) -> V14 tmp10         (last use)
                                                            ▌    simd16 field V00._fld2 (fldOffset=0x10) -> V15 tmp11         (last use)
                                                            ▌    simd16 field V00._fld3 (fldOffset=0x20) -> V16 tmp12         d2 (last use) REG NA,d1,d2 $540
                                                            ┌──▌  t90    struct
N449 ( 10,  7) [000091] -----------                         ▌  RETURN    struct REG NA $VN.Void
```
(note the REG NA instead of REG d0).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add memory fences to ByRefAssignRef on Unix ARM64 dotnet/runtime
2 participants
@MichalStrehovsky @AntonLapounov