[PATCH] D22057: Prevent devirtualization of calls to un-instantiated functions.

[Sunil Srivastava via cfe-commits]

Sunil_Srivastava updated this revision to Diff 71432.
Sunil_Srivastava added a comment.

This is an update to address points raised by Richard Smith:

1. The bit and the access functions have been renamed from MarkedForPendingInstantiation to InstantiationIsPending.
2. It closely, though not entirely, tracks whether the function is on the PendingInstantiations list. More on this point below.
3. The test explicitly allows devirtualization if Function->isDefined(). This is also needed by the change in point 2 above.
4. The test has been enhanced to have PCH tests.

Now: Why the InstantiationIsPending bit is not precisely tracking the presence in the PendingInstantiations list?

Basically this is because I think that the call to Func2 in the test SHOULD get devirtualized. Func2 is not defined in this TU, an uncommon but possible situation. Given that, the compiler had no way to instantiate it, and it is a user error if Func2 does not get instantiated somewhere else. If Func2 does get instantiated somewhere else, then it is safe to devirtualize calls to it.

In contrast, operator[] is defined by the user, but for some reason (which will be removed by my next checkin, in situations we know of) the compiler has decided to not instantiate it (or rather, not decided to instantiate it, to be precise). In this case we do not want to devirtualize call to it, because the definition is not required to exist somewhere else. The whole motivation of this commit is to prevent such calls from devirtualization.

The instantiation loop removes items from the PendingInstantiations list and instantiates them, if the body is present. In the case of Func2, the body is not present, the function has already been removed from the list, yet it is not isDefined(). We need some way to distinguish this from the contrasting case of operator[], where the function was never put on the PendingInstantiations list. Hence in cases like Func2, I am not unsetting the InstantiationIsPending bit at the time of its removal from the list. Loosely speaking, we can say that the instantiation is indeed pending, though in some other TU.

Comments in Decls.h explain this behavior, though not in such details.

The expected behavior of the test will change by my next “first fix” commit, of course; in that the operator[] will get instantiated, and the call will be devirtualized.


https://reviews.llvm.org/D22057

Files:
  include/clang/AST/Decl.h
  lib/CodeGen/CGClass.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  test/CodeGen/no-devirt.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D22057.71432.patch
Type: text/x-patch
Size: 7783 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160914/87267ed3/attachment.bin>