[cranelift-codegen] Feature Request: Support for Rust-like Memory Ordering in Atomic IR Instructions in InstBuilder

[ahqsoftwares]

Feature

Currently, Cranelift's atomic instructions (atomic_load, atomic_store, atomic_rmw, and atomic_cas) are hardcoded to Sequentially Consistent (SeqCst).

Even though it is correct for the current WebAssembly Threads proposal, it results in a performance bottleneck for non-Wasm frontends like rustc_codegen_cranelift or custom JITs that might require C11-style weak memory ordering.

Proposed Change

Addition of a MemoryOrdering enum to the following:

• atomic_load
• atomic_store
• atomic_rmw
• atomic_cas

Suggested Ordering of enum

enum MemoryOrdering {
    Relaxed,
    Acquire,
    Release,
    AcqRel,
    #[default]
    SeqCst, // Default for backward compatibility
}

Implementation

• Updating the CDSL in cranelift-codegen/meta
• Updating ISLE

Alternatives

• FFI/Libcalls: High overhead due to context switching and defeats the purpose of MemoryOrdering.
• Inline Assembly: Brittle, defeats the purpose of a portable IR.

Please Note

I am currently only focusing on the high level IR representation. I am not much acquainted with the spefic ISA lowering logic and hence, leave it to those more familiar than I am.

[cfallin]

I'd be happy to review a PR for this. Note that Cranelift's design today would permit an ordering field to affect the lowering (causing e.g. different fences or variants of atomic instructions to actually be emitted) but would not take advantage of the weaker ordering in alias analysis or code motion, as e.g. LLVM and gcc do, because Cranelift never performs code motion on memory operations. So the actual performance gains would be marginal at best, especially e.g. on x86 where default loads/stores already have Release/Acquire semantics due to TSO.

[ahqsoftwares]

Thanks a lot for the feedback @cfallin
I am confortable in implementing the enum in CDSL and updating metadata/ISLE to carry the field.

However, I don't have the expertise to optimize the lowering so at best I would be able to alias them to the current SeqCst implementation, i.e. providing the hooks to implement the specialized fences.

Would such a PR focusing mainly on the IR Infrastructure work?