[PATCH] D65403: [COFF, ARM64] Reorder handling of aarch64 MSVC builtins

[David Major via Phabricator via cfe-commits]

dmajor marked an inline comment as done.
dmajor added inline comments.


================
Comment at: lib/CodeGen/CGBuiltin.cpp:8182
   switch (BuiltinID) {
   default: return nullptr;
   case NEON::BI__builtin_neon_vbsl_v:
----------------
efriedma wrote:
> I'm a little concerned about the overall code structure here; even if moving the code for the MSVC builtins is enough to fix those builtins specifically, is it actually impossible to hit this "default"?  If it is, can we convert it to an "unreachable"?
I'm not sure if this question was directed to me... this was a drive-by patch from my end so I'm not familiar with what other types of builtins there might be.

I should probably mention that I'm hoping to get a fix merged to 9.0 in order to unblock Firefox. Unless someone can tell me that the unreachable is definitely safe, I'd worry about adding instability into the release branch. Perhaps the unreachable could be done in a separate commit only on 10.0 trunk where the tolerance for surprises is generally better.



Repository:
  rC Clang

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

https://reviews.llvm.org/D65403