[PATCH] D85882: [AMDGPU] Update subtarget features for new target ID support

[Austin Kerbow via Phabricator via llvm-commits]

kerbowa added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp:91
   // unset everything else if it is disabled
+  TargetID.emplace(*this);
 
----------------
t-tye wrote:
> Is moving constructing TargetID to the constructor of GCNSubtarget equivalent? Was it originally put in GCNSubtarget::initializeSubtargetDependencies because the base classes were modified between the construction of GCNSubtarget and calling its GCNSubtarget::initializeSubtargetDependencies? I know this was all rather contorted logic that made it hard to reason about.
I think it is functionally equivalent. The static parts of the FeatureBits that determine whether a feature is supported are constructed early before initializeSubtargetDependencies is called. We can use this to have an initial value of 'Unsupported' or 'Any' for TargetID, and then do more processing based on the feature string when initializeSubtargetDependencies is called later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85882