[PATCH] D89622: [COFF][ARM] Fix CodeView for Windows on 32bit ARM targets.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

Thanks, this is a frequently reported issue.



================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:129
   case Triple::ArchType::thumb:
-    return CPUType::Thumb;
+    return Triple.getOS() == Triple::Win32 ? CPUType::ARMNT : CPUType::Thumb;
   case Triple::ArchType::aarch64:
----------------
compnerd wrote:
> I think that `Triple.isOSWindows()` would be nicer than the explicit check to Win32.
Is this conditional necessary? Can we always use ARMNT? Under what circumstances would one with to emit codeview for a non-windows OS triple?


================
Comment at: llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp:161
+    break;
+  default:
+    CpuType = CPUType::X64;
----------------
compnerd wrote:
> It might be nice to change this to explicitly:
> 
> ```
> case COFF::IMAGE_FILE_MACHINE_AMD64:
> ```
> 
> and add the x86 case and `llvm::fatal_error` in any other case.
For a dumper, I would prefer that the tool not report a fatal error for an unknown machine type. I think it would be best to send the code to the `enumFallback<Hex16>` case below, so the registers come out as numbers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89622