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

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

DHowett-MSFT updated this revision to Diff 141055.
DHowett-MSFT added a comment.

Switched from `i686-windows` to `i686-unknown-windows-msvc` as per @compnerd's suggestion.
Based on the prior discussion around MSVC's definition of `va_{start,end,arg}` and how it is not a valid implementation that we do not pattern-match and convert into the intrinsid, I do not see the need in implementing an MSVC `va_start` test.

Regardless of that, the behaviour under test is that the function is _not inlined_, not that the intrinsic is emitted properly.


Repository:
  rC Clang

https://reviews.llvm.org/D44646

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/ms-forceinline-on-variadic.cpp


Index: test/Sema/ms-forceinline-on-variadic.cpp
===================================================================
--- /dev/null
+++ test/Sema/ms-forceinline-on-variadic.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -emit-llvm -o - -triple i686-unknown-windows-msvc -verify -fms-extensions %s \
+// RUN:            | FileCheck %s
+//
+// (instruction scheduling crash test)
+// RUN: %clang_cc1 -S -o - -triple i686-unknown-windows-msvc -fms-extensions %s
+
+inline __attribute__((always_inline)) void attribute_variadic(int f, ...) { } // expected-warning {{inlining attribute 'always_inline' ignored on variadic function}}
+
+__forceinline inline void msvc_variadic(int f, ...) { // expected-warning {{inlining attribute '__forceinline' ignored on variadic function}}
+// CHECK-DAG: define {{.*}} void [[MSVC_VARIADIC:@".*msvc_variadic.*"]](
+    __builtin_va_list ap;
+    __builtin_va_start(ap, f);
+    __builtin_va_end(ap);
+}
+
+struct a {
+    // members are, by default, thiscall; enforce it for the purposes of the test
+    void __thiscall dispatcher();
+};
+
+void __thiscall a::dispatcher() {
+    msvc_variadic(1, 2, 3);
+// CHECK-DAG: define dso_local x86_thiscallcc void @"{{.*dispatcher.*}}"(
+// CHECK-DAG: call void {{.*}} [[MSVC_VARIADIC]]
+}
+
+void t() {
+    a{}.dispatcher();
+}
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -3886,6 +3886,12 @@
     return nullptr;
   }
 
+  if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+      hasFunctionProto(D) && isFunctionOrMethodVariadic(D)) {
+    Diag(Range.getBegin(), diag::warn_always_inline_on_variadic) << Ident;
+    return nullptr;
+  }
+
   if (D->hasAttr<AlwaysInlineAttr>())
     return nullptr;
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2694,6 +2694,9 @@
 def warn_attribute_after_definition_ignored : Warning<
   "attribute %0 after definition is ignored">,
    InGroup<IgnoredAttributes>;
+def warn_always_inline_on_variadic : Warning<
+  "inlining attribute %0 ignored on variadic function">,
+   InGroup<IgnoredAttributes>;
 def warn_unknown_attribute_ignored : Warning<
   "unknown attribute %0 ignored">, InGroup<UnknownAttributes>;
 def warn_cxx11_gnu_attribute_on_type : Warning<


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44646.141055.patch
Type: text/x-patch
Size: 2466 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180404/de325ee6/attachment-0001.bin>