DHowett-MSFT added inline comments.
================ Comment at: test/Sema/ms-forceinline-on-variadic.cpp:14 + __builtin_va_end(ap); +} + ---------------- efriedma wrote: > compnerd wrote: > > DHowett-MSFT wrote: > > > compnerd wrote: > > > > Would be nice to have a second test that uses the Microsoft definitions > > > > (`char *` and the offsetting handling for the `va_list` since when > > > > building against the Windows SDK, that is the way that `va_list` and > > > > the `va_*` family of functions will get handled. > > > Should/do those still result in the intrinsic being emitted? If not, LLVM > > > shouldn't have trouble during instruction scheduling, but inlining may > > > still be broken. Regardless, though, this test exists only to make sure > > > the function doesn't end up truly inlined, regardless of its contents. > > That would still be lowered with the intrinsics. The intent is to make > > sure that the different implementation does get lowered appropriately. > To get correct lowering in LLVM, the va_start macro *must* be translated to > __builtin_va_start(); otherwise, the generated IR is nonsense and you'll > miscompile. (See also https://bugs.llvm.org/show_bug.cgi?id=24320 .) This sounds like an argument for not including a test of the Microsoft definition of `va_start`. Repository: rC Clang https://reviews.llvm.org/D44646 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
