Adding support for the X86Base.Pause intrinsic on Mono

[tannergooding]

This resolves#61693

[tannergooding]

CC. @fanyang-mono, @EgorBo

What's the correct CI leg that needs to be kicked off to validate things here?

[lambdageek]

I don't think this is enough. This only adds a mini opcode and an LLVM lowering for it.

You also need at least:

• Add something to simd-methods.h

  runtime/src/mono/mono/mini/simd-methods.h

  Line 258 in a1b391b

  METHOD(BitScanReverse)

• Add something to simd-intrinsics.c

  runtime/src/mono/mono/mini/simd-intrinsics.c

  Line 2312 in a1b391b

  {SN_BitScanReverse},

  and

  runtime/src/mono/mono/mini/simd-intrinsics.c

  Line 2919 in a1b391b

  caseSN_BitScanForward:

• Implement non-LLVM JIT/AOT support for the new mini opcode

Actually, the new mini opcode isn't necessary. There's already OP_RELAXED_NOP (used for SpinWait_nop) which is lowered to SSE2 pause on x86/amd64

[tannergooding]

Actually, the new mini opcode isn't necessary. There's already OP_RELAXED_NOP (used for SpinWait_nop) which is lowered to SSE2 pause on x86/amd64

I don't think its a good idea to rely on OP_RELAXED_NOP always being lowered to pause. We don't want a RELAXED_NOP, we want exactly X86Base.Pause. There should be no exceptions to this and future changes shouldn't impact it.

Implement non-LLVM JIT/AOT support for the new mini opcode

I'll add the additional handling for the non-LLVM path.

[lambdageek]

I'll add the additional handling for the non-LLVM path.

Hmm... now I'm not sure... the x86base group is not marked as JIT supported in simd-intrinsics. So maybe it's fine for this to be LLVM-only? a bit surprising since it seems like we should be able to support these instructions in the JIT, too.

[tannergooding]

Updated to match what SFENCE/LFENCE/MFENCE do.

I was going to also add support for ArmBase.Yield, but it looks like the support isn't quite there and its a more complex fix.

For the JIT, the actual instruction encoding is D503 203F and so, theoretically, this is:

#define arm_yield(p) arm_hint ((p), 0x1)

For LLVM, it'd be aarch64_hint with a value of 1, so roughly:

Function *F = CGM.getIntrinsic(Intrinsic::aarch64_hint);
return Builder.CreateCall(F, llvm::ConstantInt::get(Int32Ty, 1));

but the overall mono logic here is different from what I'm familiar with and I'm not quite sure what the "right" fix is.

[vargaz]

Most intrinsics in simd-intrinsic.c are only supported with llvm right now.

[fanyang-mono]

/azp runtime-staging-manual

[azure-pipelines]

Command 'runtime-staging-manual' is not supported by Azure Pipelines.

Supported commands

• help:
• Get descriptions, examples and documentation about supported commands
• Example: help "command_name"
• list:
• List all pipelines for this repository using a comment.
• Example: "list"
• run:
• Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
• Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
• where:
• Report back the Azure DevOps orgs that are related to this repository and org
• Example: "where"

See additional documentation.

[fanyang-mono]

/azp run runtime-staging-manual

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[fanyang-mono]

Lane runtime-staging-manual (Build Android x86 Release AllSubsets_Mono) should be able to qualify the change here.

[agocke]

FYI, I've disabled the test in #61743 for now. If you merge in main you should be able to re-enable it in this branch.

[fanyang-mono]

Lane runtime-staging-manual (Build Android x86 Release AllSubsets_Mono) should be able to qualify the change here.

Actually, what I sad earlier was not true. Since the newly added intrinsics will only be added for LLVM and X86Base.Pause is really for all intel architectures. So to qualify this change, Mono llvmaot Pri0 Runtime Tests Run Linux x64 release needs to run.

[imhameed]

I'll add the additional handling for the non-LLVM path.

Hmm... now I'm not sure... the x86base group is not marked as JIT supported in simd-intrinsics. So maybe it's fine for this to be LLVM-only?

It's fine for this to be LLVM-only for now.

a bit surprising since it seems like we should be able to support these instructions in the JIT, too.

Adding bsf, bsr, and rep nop/pause to mini-amd64.c shouldn't be too bad, although I don't think it's very urgent.

[fanyang-mono]

/azp run runtime-manual

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[tannergooding]

/azp run runtime-manual

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[tannergooding]

Can this be merged. It looks like all the failures are Android/iOS/WASM and are unrelated.

[vargaz]

This does work for me locally:

0000000000000025	pause
0000000000000027	popq	%rax
0000000000000028	retq

but only when running with --llvm

[fanyang-mono]

Followup: Mono llvmaot lane passed on rolling build.

[fanyang-mono]

Followup: Mono llvmaot lane passed on rolling build.

By the time that I exam the rolling build tests results, this change has been reverted. My previous comment could be ignored.