I think this would be clearer (and not too fragile) if you match the whole thing end-to-end, i.e.

(Could also match the argument names from the signature instead of hardcoding %a and %v, but I think it's fine, it's pretty unlikely that LLVM will start changing/dropping them in the future.)

This then leads to the question--why do we roundtrip %v through inttoptr/ptrtoint?

I suppose this is because in MIR the atomic op takes two pointer arguments, but does it really make sense to add two pointers?

Correct, adding two pointers doesn't make sense. The problem is that we're implementing an operation which takes a pointer operand and an integer operand. There's no atomic ptradd.

In terms of provenance, this is cursed if there are two pointer operands because which provenance is written back through the pointer? It's also cursed if we do this with integers, because then haven't we written an integer over a pointer, wiping the provenance from the memory? (I'm sort of hand-waving the kind of provenance semantics in Miri over LLVM IR which is itself a bit dubious)

Right, that makes sense. I should've been more explicit--what I mean is, the relevant part of the MIR for your example currently looks like:

fn atomicptr_fetch_byte_add(_1: &AtomicPtr<u8>, _2: usize) -> *mut u8 {
  ...
  _4 = move _7 as *mut *mut u8 (PtrToPtr);
  _6 = _2 as *mut u8 (Transmute);
  _3 = atomic_xadd_relaxed::<*mut u8>(move _4, move _6) -> [return: bb1, unwind unreachable];

i.e. the intrinsic we're implementing here does take two pointer operands. Isn't that wrong? (Well, maybe not wrong but suboptimal.)

In what way would an intrinsic based on a pointer to an integer and and an integer operand be better? Or are you suggesting that we add an intrinsic fn atomic_ptradd_relaxed(*mut *mut T, usize)?

Or are you suggesting that we add an intrinsic fn atomic_ptradd_relaxed(*mut *mut T, usize)?

Yes. (And the same for every atomic operation on pointers (xor, or, etc.) that only makes sense with an integer second operand.)

Given that the only sensible operation here is offsetting a pointer by an integer, wouldn't it be better for the intrinsic to match that?

(I guess this isn't so much a problem because we can just define atomic_xadd_relaxed::<*mut u8> to always use the provenance of the first argument, and I assume this is what Miri implements. Having to convert the offset to/from a pointer "just" makes the codegen more complex.)

always use the provenance of the first argument, and I assume this is what Miri implements.

Yes indeed.

But ideally we'd convince LLVM to provide an intrinsic that takes (*mut *mut T, usize); that is the only way to avoid pointer-integer transmutation.

Right, we need the LLVM change regardless, I don't mean to imply that this would solve the provenance issue. It would just remove some other unnecessary weirdness.

It doesn't need to be done as part of this PR though, I can do it separately.

Agree with @erikdesjardins that the Rust intrinsic should accept usize rather than pointer for the second arg.

Are the extra arguments necessary?

Whoops. Leftover from a previous idea.