[PATCH] [MSVC] Improved __noop support (https://llvm.org/bugs/show_bug.cgi?id=14081)

[Reid Kleckner]

I'm concerned that there are other ways for code inside __noop() to trigger template instantiation, which will cause us to do semantic analysis in a weird state.


================
Comment at: lib/Sema/SemaExpr.cpp:13051
@@ -13050,2 +13050,3 @@
       OdrUse = false;
-  MarkExprReferenced(*this, E->getLocation(), E->getDecl(), E, OdrUse);
+  // If we are insidecall of __noop intrinsic we shouldn't match function as
+  // referenced. We do so not to instantiate template functions. However we
----------------
s/match/mark/

================
Comment at: lib/Sema/SemaExpr.cpp:13055
@@ +13054,3 @@
+  // This behaviour is the same as Microsoft compiler behaviour.
+  if (!InsideNoop || !isa<FunctionDecl>(E->getDecl()))
+    MarkExprReferenced(*this, E->getLocation(), E->getDecl(), E, OdrUse);
----------------
Why do we need to mark non-FunctionDecls referenced? Surely they can trigger template instantiation of other function decls.

================
Comment at: lib/Sema/SemaExpr.cpp:13055
@@ +13054,3 @@
+  // This behaviour is the same as Microsoft compiler behaviour.
+  if (!InsideNoop || !isa<FunctionDecl>(E->getDecl()))
+    MarkExprReferenced(*this, E->getLocation(), E->getDecl(), E, OdrUse);
----------------
rnk wrote:
> Why do we need to mark non-FunctionDecls referenced? Surely they can trigger template instantiation of other function decls.
Can this just be `if (InsideNoop) return;` at the beginning of the function?

================
Comment at: test/CodeGen/builtin-ms-noop.cpp:35
@@ +34,3 @@
+// CHECK: define void @j()
+// CHECK: store i32* null, i32** %A
+}
----------------
This test will not pass in NDEBUG mode because the IR names are skipped. You can either pattern match the alloca, or you can store to a global.

http://reviews.llvm.org/D9000

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/