[PATCH] D102334: [AMDGPU] Sort LDS globals based on thier size and alignment.

[Mahesha S via Phabricator via llvm-commits]

hsmhsm added a comment.

In D102334#2756371 <https://reviews.llvm.org/D102334#2756371>, @foad wrote:

>> [AMDGPU] Sort LDS globals based on thier size and alignment.
>
> Typo "their" :-)
>
>> In order to increase the probability of aligned accessing of LDS memory
>> operations,
>
> Are you expecting this to affect instruction selection? Or just to increase the probability that the selected instructions will go fast because they will see an aligned address at run time?

It is to  increase the probability that the selected instructions will go fast because they will see an aligned address at run time.

>> sort LDS globals based on thier size and alignment, and then
>> allocate memory in that sorted order.
>
> Sorting seems like an odd way  to do this, because it *might* happen to over-align a global, but only by chance depending on what other globals get sorted before it.

Lately, I am actually of same openion - sorting will definetely help us reduce space overhead due to padding, but may not be with alignment.

> Instead how about explicitly over-aligning all globals with something like:
>
>   for each lds global:
>     if size > 8:
>       // we might want to use a b96 or b128 load
>       alignment = max(alignment, 16)
>     else if size > 4:
>       // we might want to use a b64 load
>       alignment = max(alignment, 8)
>     else if size > 2:
>       // we might want to use a b32 load
>       alignment = max(alignment, 4)
>     else if size > 1
>       // we might want to use a b16 load
>       alignment = max(alignment, 2)
>
> (You could still sort them after doing that, but the only reason would be to try to minimise wasted space due to alignment gaps.)

Looks like a good suggestion to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102334