[PATCH] D102401: [AMDGPU] Allocate LDS globals in sorted order of their alignment and size.

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp:92-93
+
+  // Sort LDS globals (which are used within kernel K) by size, descending, and
+  // then, by alignment, descending. On ties, sort by name, lexicographical.
+  llvm::stable_sort(
----------------
rampitec wrote:
> hsmhsm wrote:
> > foad wrote:
> > > Why is sorting by size important?
> > > 
> > > I can see that sorting by alignment, descending, avoids alignment gaps (at least in the common case where an object's size is a multiple of its alignment). But sorting by size //first// means that most globals //won't// be sorted by alignment (unless they happened to have the same size and different alignments), so you won't get that benefit.
> > I think we need to first sort based on "alignment", then on tie, based on size.
> Probably yes. Strategy to sort by size was used along with the super-alignment, so did the same thing.
I still don't understand if there's a reason to sort by size or name here. It's a stable sort, so why not just sort by alignment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102401