[PATCH] D76357: [AMDGPU] Implement wave32 DWARF register mapping

[Venkata Ramanaiah Nalamothu via Phabricator via llvm-commits]

RamNalamothu marked 6 inline comments as done.
RamNalamothu added inline comments.


================
Comment at: llvm/unittests/MC/AMDGPU/DwarfRegMappings.cpp:44
+
+  return std::unique_ptr<const MCRegisterInfo>(TM->getMCRegisterInfo());
+}
----------------
arsenm wrote:
> This isn't creating an owning reference? Why is this unique ptr?
Thanks for catching this. Fixed the memory leak.


================
Comment at: llvm/unittests/MC/AMDGPU/DwarfRegMappings.cpp:49
+  // Test triple OS strings
+  std::vector<std::string> OSTestValues = {"", "amdhsa", "amdpal"};
+
----------------
arsenm wrote:
> The multiple OS tests aren't needed, my point was it should just be set to something sensible other than the defaulted host OS
May be not needed but they do test few OS flavors for sanity and I guess there is no harm in having those tests.


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

https://reviews.llvm.org/D76357