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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h:303
 
+  Optional<AMDGPU::IsaInfo::AMDGPUTargetID> TargetID;
+
----------------
t-tye wrote:
> arsenm wrote:
> > kerbowa wrote:
> > > arsenm wrote:
> > > > Don't see why this would be Optional if the query just crashes if it's missing
> > > It's not constructed until "initializeSubtargetDependencies" is called.
> > It can just use an invalid target ID value then
> But optional is a better way of indicating an object does not yet have a value than imposing that the value itself has a distinguished NULL value.
There's no reason to use Optional here since None is not a meaningful state. This is only an artifact of the initialization order


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85882