[PATCH] D128014: [AMDGPU] Improve assembler + disassembler handling of kernel descriptors

[Scott Linder via Phabricator via llvm-commits]

scott.linder marked 5 inline comments as done.
scott.linder added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h:100
   mutable bool HasLiteral;
+  mutable Optional<bool> EnableWavefrontSize32;
 
----------------
rochauha wrote:
> MaskRay wrote:
> > Can this be a plain `bool`?
> I think using `Optional` is appropriate. The `.amdhsa_wavefront_size32` directive and consequently `EnableWavefrontSize32` here are specific to GFX10-11. Otherwise that bit is reserved and must be 0 for older hardware.
Right, like Ronak says the nominal type for this parameter to `AMDGPU::IsaInfo::getVGPREncodingGranule` is `Optional<bool>`, and using plain `bool` here would be assuming some details about the implementation in `IsaInfo` for limited benefit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128014