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

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp:20
+template <typename R, typename E>
+static bool contains(R &&VMap, const E &Element) {
+  return VMap.find(Element) != VMap.end();
----------------
Isn't this llvm::is_contained?


================
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(
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102401