[Mesa-dev] [PATCH] swr/rast: Correctly allocate SWR_STATS memory as cacheline aligned

[Emil Velikov]

On 6 July 2017 at 19:55, Tim Rowley <timothy.o.rowley at intel.com> wrote:
> Cacheline alignment of SWR_STATS to prevent sharing of cachelines
> between threads (performance).
>
> Gets rid of gcc-7.1 warning about using c++17's over-aligned new
> feature.
>
Thank you!

> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/gallium/drivers/swr/rasterizer/core/api.cpp     | 6 +++---
>  src/gallium/drivers/swr/rasterizer/core/threads.cpp | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/src/gallium/drivers/swr/rasterizer/core/api.cpp b/src/gallium/drivers/swr/rasterizer/core/api.cpp
> index d3d80e4..087a24a 100644
> --- a/src/gallium/drivers/swr/rasterizer/core/api.cpp
> +++ b/src/gallium/drivers/swr/rasterizer/core/api.cpp
> @@ -108,7 +108,7 @@ HANDLE SwrCreateContext(
>      CreateThreadPool(pContext, &pContext->threadPool);
>
>      pContext->ppScratch = new uint8_t*[pContext->NumWorkerThreads];
> -    pContext->pStats = new SWR_STATS[pContext->NumWorkerThreads];
> +    pContext->pStats = (SWR_STATS*)AlignedMalloc(sizeof(SWR_STATS) * pContext->NumWorkerThreads, 64);
>
>  #if defined(KNOB_ENABLE_AR)
>      // Setup ArchRast thread contexts which includes +1 for API thread.
> @@ -363,7 +363,7 @@ void SwrDestroyContext(HANDLE hContext)
>      // free the fifos
>      for (uint32_t i = 0; i < KNOB_MAX_DRAWS_IN_FLIGHT; ++i)
>      {
> -        delete[] pContext->dcRing[i].dynState.pStats;
> +        AlignedFree(pContext->dcRing[i].dynState.pStats);
>          delete pContext->dcRing[i].pArena;
>          delete pContext->dsRing[i].pArena;
>          pContext->pMacroTileManagerArray[i].~MacroTileMgr();
> @@ -388,7 +388,7 @@ void SwrDestroyContext(HANDLE hContext)
>      }
>
>      delete[] pContext->ppScratch;
> -    delete[] pContext->pStats;
> +    AlignedFree(pContext->pStats);
>
>      delete(pContext->pHotTileMgr);
>
> diff --git a/src/gallium/drivers/swr/rasterizer/core/threads.cpp b/src/gallium/drivers/swr/rasterizer/core/threads.cpp
> index e03632b..36710bf 100644
> --- a/src/gallium/drivers/swr/rasterizer/core/threads.cpp
> +++ b/src/gallium/drivers/swr/rasterizer/core/threads.cpp
> @@ -363,7 +363,7 @@ INLINE void UpdateClientStats(SWR_CONTEXT* pContext, uint32_t workerId, DRAW_CON
>      }
>
>      DRAW_DYNAMIC_STATE& dynState = pDC->dynState;
> -    SWR_STATS stats{ 0 };
> +    OSALIGNLINE(SWR_STATS) stats{ 0 };
>
Sipping my morning coffee so I might be off:

The struct declaration itself already has the OSALIGNLINE, so there
should be no need to add it for the variable instance.
Did you add it to make it clearer that it has alignment?

-Emil