[PATCH] D139468: AMDGPU: Clean up LDS-related occupancy calculations

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp:146
+
+  if (AMDGPU::isGFX10Plus(*this) &&
+      !getFeatureBits().test(AMDGPU::FeatureCuMode))
----------------
nhaehnle wrote:
> arsenm wrote:
> > I'd like to avoid growing the number of generation based feature checks in favor of separate subtarget features
> Hmm. I was using the same kind of check as what's already in AMDGPUBaseInfo. And it's not clear to me what the feature here would be that makes sense.
> 
> It is unfortunate that CuMode was picked out as the feature instead of WgpMode. Should we add CuMode as a feature to everything <gfx10? Or somehow figure out how to invert the sense of the feature, and make WgpMode the feature that has to be added explicitly? Or something else entirely?
I'd prefer inverting it but that's probably a good bit of migration pain.

The current feature is the dynamic switch; I was thinking another feature that says it has the dynamic switch. That's what we do for sramecc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139468