[PATCH] D104264: Fix atomic loads and stores of 64-bit values in non-default address spaces on ARM

Tue Jun 15 07:34:24 PDT 2021

rengolin added a comment.

In D104264#2819207 <https://reviews.llvm.org/D104264#2819207>, @koutheir wrote:

> The use of a different address space in my case is used to distinguish "pointers to native memory" from "pointers to managed objects" in a Java virtual machine.

I see. I imagine to help with garbage collecting.

>> This patch assumes that ARM synchronization semantics are the same irrespective of the address space.

I believe this is true, but that's not what I was concerned about.

> I think that atomic loads and stores from non-default address spaces should behave uniformly irrespective of the integer bit width (as long as it is supported by the processor).

I also agree with this statement, so we're on the same page.

My worry wasn't about the semantics of the atomic load/store instructions, but about the introduction of the unchecked `CreateAddrSpaceCast` in there.

Basically, what that's saying is that *all* access to *any* address space is the same as an access through the default address space.

It may be true for your case, but not necessarily others. Your code would make it true for all.

I looked at other targets and none of them assume this is true, except the GPU targets on very specific conditions (I'm, assuming ABI related).

This looks to me like the wrong place to fix it. It'd probably be a matter for the middle-end casting the address space to default when performing the atomic load?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104264/new/

https://reviews.llvm.org/D104264