[llvm-branch-commits] [AutoUpgrade][AMDGPU] Adjust AS7 address width to 48 bits (PR #137418)

[Alexander Richardson via llvm-branch-commits]

arichardson wrote:

> So, `ptr addrspace(7)` is, in your terms `{{flags, length, [stride == 0], base}, offset}` where that inner struct is a `ptr addrspace(8)`. p9 is `{{flags, length, stride, base}, index, offset}`.
> 
> The common usage pattern I'd expect for ptr addrspace(7) is to make `base` there a sufficiently-aligned base pointer to your image/matrix/..., and then take offssets from there - for nothing else, you can only go 32 bits away from that base, the base (unlike the offset) has to be subgroup-uniform (it lives in scalar registers), and also, the `offset` there is (contrary to LLVM, but it's fine) unsigned.
> 
> ( Also @piotrAMD @jayfoad and anyone else they want to call in for the LLPC peerspective, since they're some of the folks who first created addrspace 7)
> 
> I completely agree with you that having ptrtoaddr be relative to the relevant address space start (which may not be NULL - amdgpu p3 has null == 0xffffffff) is the way to go here: I'm not advocating for "relative to the start of the object" here.

Perfect, this sounds good to me! One thing I still need to figure out is how to deal with KnownBits: right now it assumes null is all zeroes and the result is all pointer bits, but really we can only infer the value of the address based on known alignment.

> 
> One tthing I'm trying to understand is what one would actually _do_ with the 48-bit underlying address that your `ptrtoaddr` would create.
> 
> Side note, one reason I want the "value of the index bits" version of ptrtoaddr is
> 
> ```c
> char memory[16] = {'a', 'b', 'c', 'd', ... 'o', 'p'};
> char*(7) A = __amdgcn_make_buffer_rsrc(memory, /*stride=*/0, /*length=*/8, [flags]);
> char*(7) B = __amdgcn_make_buffer_rsrc(memory + 8, 0, 8, [flags]);
> printf("A[8]: %c, B[0]: %c\n" A[8], B[0]); // Prints "A[8]: ^@ B[0]: h" - that is, that oob read gives 0
> assert(A[8] != B[0]); // should be true, to my knowledge of C semantics - one-past-A isn't B
> ```
> 
> This out of bounds behavior is one of the reasons I think that just using the bits effected by GEP as the "ptrtoaddr" result is more useful. It does mean that `==` might sometimes return false when two buffers do actually encompass the same memory location, but since we already have to break _something_, I'd rather lose the unusual case of aliased buffer descriptors

If your assert is supposed to be `assert((void*)&A[8] != (void*)&B[0])` then that is defined by the C standard as returning equal (6.5.9 Equality operators):

> Two pointers compare equal if and only if both are null pointers, both are pointers to the same object (including a pointer to an object and a subobject at its beginning) or function, both are pointers to one past the last element of the same array object, or **one is a pointer to one past the end of one array object and the other is a pointer to the start of a different array object that happens to immediately follow the first array object in the address space**.

If you mean to dereference in the assert then anything goes since accessing out-of-bounds of buffer is UB.

Another thing to note is that the C standard is quite vague on subobjects, but the way I would interpret it is that your `A` and `B` examples are essentially restricted 8-byte sub-objects of the 16-byte `memory` object. This is very similar to how CHERI allows for sub-object bounds. For CHERI the only difference is that by default the dereference of `A[8]` would trap unless your trap handler emulates it and gives you a fixed value. The CHERI equivalent would be:

```c
char memory[16] = {'a', 'b', 'c', 'd', ... 'o', 'p'};
char* A = __builtin_cheri_bounds_set(memory, /*length=*/8);
char* B = __builtin_cheri_bounds_set(memory + 8, /*length=*/8);
printf("A[8]: %c, B[0]: %c\n" A[8], B[0]); // traps, the OOB read raises an exception
assert((void*)&A[8] != (void*)&B[0]); // C standard suggests this should return equal and fail the assert
```

One reason I like the linear offset from start of the address space interpretation is that it means we can use it for the C semantics of comparing just the address


https://github.com/llvm/llvm-project/pull/137418