[PATCH] D146031: [AMDGPU] Add MMOs for GFX11 Streamout Instructions

[Diana Picus via Phabricator via llvm-commits]

rovka added inline comments.


================
Comment at: llvm/docs/AMDGPUUsage.rst:678
      Buffer Fat Pointer (experimental) 7               *TODO*
+     Streamout Registers               8               *TODO*      GS_REGS
      ================================= =============== =========== ================ ======= ============================
----------------
foad wrote:
> arsenm wrote:
> > foad wrote:
> > > arsenm wrote:
> > > > foad wrote:
> > > > > My gut feeling is that this should be an internal implementation detail of the compiler, should never be exposed to the user, so should not be documented here.
> > > > > 
> > > > > Adding @nhaehnle.
> > > > I do think we need to document any address space numbers used for any purpose. We should also probably reserve a range of address spaces for external software uses 
> > > I was thinking this would be completely inaccessible from IR and only introduced during instruction selection. Do you agree? Do you still think it needs to be documented here? Why?
> > It would still need to be documented as internally used for something so it doesn't get reused for something else
> But we can reuse the number for something else whenever we like - we can just renumber STREAMOUT_REGISTER to something different, with no externally visible effect.
I kind of see the point that we don't really have an "LLVM IR Address Space Number" for this, since it doesn't show up in the IR. We could just document that there IS an address space for the Streamout registers (if indeed we decide we need one), but don't assign it a formal address space number in this doc.


================
Comment at: llvm/docs/AMDGPUUsage.rst:788
+**Streamout Registers**
+  Dedicated registers used by the GS NGG Streamout Instructions.
+
----------------
t-tye wrote:
> My understanding is that address spaces are about memory, not registers. So why is an address space being used for this purpose? Is it that the values are being passed in memory?
I think the idea is that these registers are embedded into the GDS. 

In principle, we could use the region address space and as far as the existing tests are concerned we'd get the same results, but the docs are pretty clear that these are not operating on GDS directly, so it seemed cleaner to have a new address space.

I'm open to other interpretations :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146031