[PATCH] D155393: [clang][Interp] Implement __builtin_isfpclass

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:225
+  const CallExpr *CE = cast<CallExpr>(E);
+  PrimType FPClassArgT = *S.getContext().classify(CE->getArgs()[1]->getType());
+  APSInt FPClassArg = peekToAPSInt(S.Stk, FPClassArgT);
----------------
Should we add an assert that `[1]` is a valid index to read from?


================
Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:232
+      static_cast<int32_t>((F.classify() & FPClassArg).getZExtValue());
+  S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Result));
+
----------------
Shouldn't this be pushing an `int`? (e.g., might not be 32 bits) -- same for the other builtins, now that I look at them.


================
Comment at: clang/lib/AST/Interp/PrimType.h:135
+    default:                                                                   \
+      llvm_unreachable("Not an integer value");                                \
+    }                                                                          \
----------------
BitInt comes to mind. ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155393