thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land.
Nice! ================ Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1019 + default: + break; + case ARM::BI_BitScanForward: ---------------- Maybe `return None` here and LLVM_UNREACHABLE at the bottom? ================ Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1165 + default: + break; + case AArch64::BI_BitScanForward: ---------------- same suggestion ================ Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1311 + default: + break; + case clang::X86::BI_BitScanForward: ---------------- Same suggestion ================ Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7175 + // Handle MSVC intrinsics before argument evaluation. + if (Optional<MSVCIntrin> MsvcIntId = translateArmToMsvcIntrin(BuiltinID)) ---------------- This comment could answer "why" too: "...because EmitSMVCBuiltinExpr() evaluates arguments and they shouldn't be evaluated twice" (maybe worded better) ================ Comment at: clang/lib/CodeGen/CodeGenFunction.h:4126 + llvm::Value *EmitEvaluatedMSVCBuiltin(MSVCIntrin BuiltinID, const CallExpr *E, + ArrayRef<llvm::Value *> Ops); ---------------- ...where's the definition of this function? I can't see calls either. I guess this is a remnant from an earlier approach? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92061/new/ https://reviews.llvm.org/D92061 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
