[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

[Dustin L. Howett via Phabricator via cfe-commits]

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