rnk added inline comments.
================ Comment at: include/clang/Basic/BuiltinsAArch64.def:65 +// Win64-compatible va_list functions +BUILTIN(__builtin_ms_va_start, "vc*&.", "nt") +BUILTIN(__builtin_ms_va_end, "vc*&", "n") ---------------- I strongly suspect that Microsoft will never adopt a varargs calling convention that uses a complex, non-`char*` va_list. I'm starting to think we should move this to the generic builtin list and make it available everywhere. The semantics are that you can only use __builtin_ms_va_start in ms_abi functions. It always produces a `char*`-style va_list. ================ Comment at: include/clang/Basic/Specifiers.h:239 CC_X86Pascal, // __attribute__((pascal)) CC_X86_64Win64, // __attribute__((ms_abi)) CC_X86_64SysV, // __attribute__((sysv_abi)) ---------------- I think we might prefer to make this non-x86_64 specific. I suspect that this pattern will arise again on ARM32 if anyone goes back there in seriousness. We'll probably want sysv_abi as well as ms_abi, and all the logic should be independent of the ISA: ms_abi is a no-op when the target OS is already Windows, and sysv_abi is a no-op when the target OS isn't Windows. ================ Comment at: lib/CodeGen/CGBuiltin.cpp:5276 const CallExpr *E) { + if (BuiltinID == AArch64::BI__builtin_ms_va_start || + BuiltinID == AArch64::BI__builtin_ms_va_end) ---------------- If we made __builtin_ms_va_start generic, that would also eliminate this duplicate code. ================ Comment at: lib/CodeGen/CGCall.cpp:223-224 if (D->hasAttr<MSABIAttr>()) - return IsWindows ? CC_C : CC_X86_64Win64; + return IsWindows ? CC_C : Arch == llvm::Triple::aarch64 ? CC_AArch64Win64 + : CC_X86_64Win64; ---------------- Unifying the ms_abi CCs would remove the need for this. ================ Comment at: lib/Sema/SemaChecking.cpp:3625 /// target and calling convention. static bool checkVAStartABI(Sema &S, unsigned BuiltinID, Expr *Fn) { const llvm::Triple &TT = S.Context.getTargetInfo().getTriple(); ---------------- Oh dear, how can we keep this simple. I think unifying the CC's would improve things. ================ Comment at: lib/Sema/SemaDeclAttr.cpp:4283 + + CC = Context.getTargetInfo().getTriple().isOSWindows() + ? CC_C ---------------- Ditto https://reviews.llvm.org/D34475 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits