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

Mon Oct 19 18:50:57 PDT 2020

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

================
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:
> rnk wrote:
> > 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?
> The particular case that I think that this is trying to account for is WinCE vs WinNT.  However, WinCE isn't currently supported by LLVM, so .... a TODO comment + ARMNT sounds like a good idea to me.
Coment + ARMNT works for me. I just added the conditional there to not break anything existing but if there's no CE support anyways it should be fine.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89622