[PATCH] D145441: [AMDGPU] Define data layout entries for buffers

Nicolai Hähnle via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 7 12:08:44 PST 2023


nhaehnle added a comment.

In D145441#4175428 <https://reviews.llvm.org/D145441#4175428>, @krzysz00 wrote:

> @foad I was trying to avoid sending in one mega-patch that has the entire prototype.

The best way to do this is to work on a branch that you rebase regularly. You can push the branch to a personal clone of llvm-project on GitHub and point folks there from e.g. Discourse, and for the purpose of Phabricator you can set up a "Stack" via the "Edit Related Revisions" option.

If you're using `arc`, then `git rebase -i -x "arc diff @^"` is one way to update a bunch of patches at once. There are probably other tools that people have written.

> These [addrspace(8) pointers] are, however, integral, since inttoptr and ptrtoint behave deterministically and reasonably.

That doesn't make sense to me. I though "integral" means that

  inttoptr(ptrtoint(p) + offset)

is the same (in terms of address, and potentially modulo provenance questions) as

  gep i8, p, offset

But that is rather untrue for addrspace(8), isn't it?



================
Comment at: llvm/docs/AMDGPUUsage.rst:794
+**Buffer Resource**
+  The buffer resource is an experemntal address space that is currently unsupported
+  in the backend. It exposes a non-integral pointer that will represent a 128-bit
----------------
Typo: experimental


================
Comment at: llvm/docs/AMDGPUUsage.rst:804-805
+
+  Casting a buffer resource to a bufer fat pointer is permitted and adds an offset
+  of 0.
+
----------------
Presumably it is only permitted if the underlying descriptor satisfies all the rules mentioned for addrspace(7).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145441



More information about the cfe-commits mailing list