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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/unittests/MC/AMDGPU/DwarfRegMappings.cpp:30
+
+  const char *Triple = "amdgcn";
+  std::string Error;
----------------
RamNalamothu wrote:
> arsenm wrote:
> > Can this be a StringRef? You also don't reuse it below. This should probably also use the full 3 component triple
> Using only the 2 component triple as the dwarf register mappings are agnostic to a particular OS (the 3rd component).
The second part doesn't do anything. Setting the OS is the interesting part since it does default to the host OS, so introduces potential variability


================
Comment at: llvm/unittests/MC/AMDGPU/DwarfRegMappings.cpp:29-38
+  InitializeTargets();
+
+  std::string Error;
+  const Target *T = TargetRegistry::lookupTarget("amdgcn-amd", Error);
+  ASSERT_TRUE(T);
+
+  TargetOptions Options;
----------------
This common setup code should be moved to a setup function, and the test should not fail on target creation failure. Otherwise this will fail when AMDGPU isn't build. unittests/CodeGen/AArch64SelectionDAGTests.cpp is a model for this


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

https://reviews.llvm.org/D76357