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.

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

[RyuJIT/arm32] CQ: enable InterlockedCmpXchg32 intrinsic #9982

Open
BruceForstall opened this issue Mar 20, 2018 · 9 comments
Open

[RyuJIT/arm32] CQ: enable InterlockedCmpXchg32 intrinsic #9982

BruceForstall opened this issue Mar 20, 2018 · 9 comments
Labels
arch-arm32 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI enhancement Product code improvement that does NOT require public API changes/additions in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@BruceForstall
Copy link
Member

BruceForstall commented Mar 20, 2018

Currently, ARM does not generate GT_CMPXCHG nodes; all other targets do.

From the importer:

// TODO-ARM-CQ: reenable treating InterlockedCmpXchg32 operation as intrinsic
case CORINFO_INTRINSIC_InterlockedCmpXchg32:

Also, in codegenarmarch.cpp, genCodeForTreeNode:

       case GT_CMPXCHG:
            NYI_ARM("GT_CMPXCHG");

category:cq
theme:hardware-intrinsics
skill-level:intermediate
cost:small

@BruceForstall
Copy link
Member Author

Related: #8626

@mikedn
Copy link
Contributor

mikedn commented Mar 20, 2018

But does ARM has the necessary instruction(s) to implement this? AFAIR I once looked at this and couldn't find anything clear. And ARM codegenlegacy is also NYI.

@mikedn
Copy link
Contributor

mikedn commented Mar 20, 2018

I see that ARM32 GCC generates a call to a library function for std::atomic_compare_exchange_strong/weak.

@BruceForstall
Copy link
Member Author

BruceForstall commented Mar 20, 2018

The right answer very well might be that this is something we won't change. In which case, we can remove these TODOs and NYI. The VM helper must do something, though, so it's a matter of "inline" that, or not.

I'm just auditing existing NYI and splitting them into separate issues.

@BruceForstall
Copy link
Member Author

Also CORINFO_INTRINSIC_InterlockedAdd32, CORINFO_INTRINSIC_InterlockedXAdd32, CORINFO_INTRINSIC_InterlockedXchg32 (GT_LOCKADD, GT_XADD, GT_XCHG). See impIntrinsic().

@sdmaclea
Copy link
Contributor

sdmaclea commented Apr 2, 2018

does ARM have the necessary instruction(s) to implement this?

Arm32 would use a load exclusive, store exclusive sequence similar to Arm64 base implementation.

@mikedn
Copy link
Contributor

mikedn commented Apr 3, 2018

Arm32 would use a load exclusive, store exclusive sequence similar to Arm64 base implementation.

Yes, but it seems that ARM32 has different instructions for this - LDREX/STREX instead of LDAXR/STLXR. Also, these instructions are only available starting with ARMv7 and only on some profiles: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dht0008a/CJHBGBBJ.html. The lack of any acquire/release semantics mention in the documentation probably means that DMB is needed as well.

@sdmaclea
Copy link
Contributor

sdmaclea commented Apr 3, 2018

@mikedn dmb is definitely needed with ldrex/strex.

ARMv7 really introduced multiprocessor coherent ARM. So I would believe ldrex/strex were part of that. It would be reasonable to assume that if we are on a multiprocessor OS, then ldrex, strex and dmb must be present.

I would guess all realistic .NET Core targets have these instructions. We can certainly pick up capabilities from the OS like we do for arm64.

@mikedn
Copy link
Contributor

mikedn commented Apr 9, 2018

So if I understand the ARM documentation correctly the generated code should look something like:

; CompareExchange(r0, r1, r2)
    dmb
LOOP:
    ldrex r3, [r0]
    cmp r3, r1
    bne DONE
    strex r4, r2, [r0]
    tst r4, r4
    bne LOOP
DONE:
    dmb
; result in r3

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-arm32 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI enhancement Product code improvement that does NOT require public API changes/additions in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants