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

2017-06-20 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL305862: Prevent devirtualization of calls to un-instantiated 
functions. (authored by ssrivastava).

Changed prior to commit:
  https://reviews.llvm.org/D22057?vs=71432=103273#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D22057

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

Index: cfe/trunk/include/clang/AST/Decl.h
===
--- cfe/trunk/include/clang/AST/Decl.h
+++ cfe/trunk/include/clang/AST/Decl.h
@@ -1656,6 +1656,7 @@
   unsigned HasImplicitReturnZero : 1;
   unsigned IsLateTemplateParsed : 1;
   unsigned IsConstexpr : 1;
+  unsigned InstantiationIsPending:1;
 
   /// \brief Indicates if the function uses __try.
   unsigned UsesSEHTry : 1;
@@ -1751,6 +1752,7 @@
 IsDeleted(false), IsTrivial(false), IsDefaulted(false),
 IsExplicitlyDefaulted(false), HasImplicitReturnZero(false),
 IsLateTemplateParsed(false), IsConstexpr(isConstexprSpecified),
+InstantiationIsPending(false),
 UsesSEHTry(false), HasSkippedBody(false), WillHaveBody(false),
 EndRangeLoc(NameInfo.getEndLoc()), TemplateOrSpecialization(),
 DNLoc(NameInfo.getInfo()) {}
@@ -1943,6 +1945,15 @@
   bool isConstexpr() const { return IsConstexpr; }
   void setConstexpr(bool IC) { IsConstexpr = IC; }
 
+  /// \brief Whether the instantiation of this function is pending.
+  /// This bit is set when the decision to instantiate this function is made
+  /// and unset if and when the function body is created. That leaves out
+  /// cases where instantiation did not happen because the template definition
+  /// was not seen in this TU. This bit remains set in those cases, under the
+  /// assumption that the instantiation will happen in some other TU.
+  bool instantiationIsPending() const { return InstantiationIsPending; }
+  void setInstantiationIsPending(bool IC) { InstantiationIsPending = IC; }
+
   /// \brief Indicates the function uses __try.
   bool usesSEHTry() const { return UsesSEHTry; }
   void setUsesSEHTry(bool UST) { UsesSEHTry = UST; }
Index: cfe/trunk/test/CodeGen/no-devirt.cpp
===
--- cfe/trunk/test/CodeGen/no-devirt.cpp
+++ cfe/trunk/test/CodeGen/no-devirt.cpp
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 %s -DUSEIT -triple %itanium_abi_triple -emit-llvm -o - |  FileCheck %s
+
+// Test with decls and template defs in pch, and just use in .cpp
+// RUN:  %clang_cc1 %s -DTMPL_DEF_IN_HEADER -triple %itanium_abi_triple -emit-pch -o %t
+// RUN:  %clang_cc1 %s -DTMPL_DEF_IN_HEADER -DUSEIT -triple %itanium_abi_triple -include-pch %t -emit-llvm -o - | FileCheck %s
+
+// Test with A in pch, and B and C in main
+// Test with just decls in pch, and template defs and use in .cpp
+// RUN:  %clang_cc1 %s -triple %itanium_abi_triple -emit-pch -o %t
+// RUN:  %clang_cc1 %s -DUSEIT -triple %itanium_abi_triple -include-pch %t -emit-llvm -o - | FileCheck %s
+
+#ifndef HEADER
+#define HEADER
+template < typename T, int N = 0 > class TmplWithArray {
+public:
+  virtual T& operator [] (int idx);
+  virtual T& func1 (int idx);
+  virtual T& func2 (int idx);
+  T ar[N+1];
+};
+struct Wrapper {
+  TmplWithArray data;
+  bool indexIt(int a) {
+if (a > 6) return data[a] ;  // Should not devirtualize
+if (a > 4) return data.func1(a); // Should devirtualize
+return data.func2(a);// Should devirtualize
+  }
+};
+
+#ifdef TMPL_DEF_IN_HEADER
+template  T& TmplWithArray::operator[](int idx) {
+  return ar[idx];
+}
+template  T& TmplWithArray::func1(int idx) {
+  return ar[idx];
+}
+#endif // TMPL_DEF_IN_HEADER
+#endif // HEADER
+
+#ifdef USEIT
+#ifndef TMPL_DEF_IN_HEADER
+template  T& TmplWithArray::operator[](int idx) {
+  return ar[idx];
+}
+template  T& TmplWithArray::func1(int idx) {
+  return ar[idx];
+}
+#endif // !TMPL_DEF_IN_HEADER
+extern Wrapper ew;
+bool stuff(int p)
+{
+  return ew.indexIt(p);
+}
+#endif
+
+// CHECK-NOT: call {{.*}} @_ZN13TmplWithArrayIbLi10EEixEi
+// CHECK-DAG: call {{.*}} @_ZN13TmplWithArrayIbLi10EE5func1Ei
+// CHECK-DAG: call {{.*}} @_ZN13TmplWithArrayIbLi10EE5func2Ei
+
Index: cfe/trunk/lib/Sema/Sema.cpp
===
--- cfe/trunk/lib/Sema/Sema.cpp
+++ cfe/trunk/lib/Sema/Sema.cpp
@@ -740,6 +740,9 @@
   // Load pending instantiations from the external source.
   SmallVector Pending;
   ExternalSource->ReadPendingInstantiations(Pending);
+  for (auto PII : Pending)
+if (auto Func = dyn_cast(PII.first))
+  Func->setInstantiationIsPending(true);
   

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

2017-02-09 Thread Richard Smith via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D22057#543199, @Sunil_Srivastava wrote:

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


[...]

> 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().

I see, so the only case in which the list and the flag differs is when we're 
doing the final instantiation step at the end of the TU? (In all other cases, 
we'd put the function back on the list for later instantiations to retry.) That 
seems fine.


https://reviews.llvm.org/D22057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2017-01-28 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment.

I have read the patch, but I don't have enough knowledge about C++ rules and 
fronted to accept it. Richard?




Comment at: lib/Sema/Sema.cpp:684
+  for (auto PII : Pending) 
+if (FunctionDecl *Func = dyn_cast(PII.first))
+  Func->setMarkedForPendingInstantiation();

Prazek wrote:
> Dry. Use auto
auto *


https://reviews.llvm.org/D22057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-12-12 Thread Sunil Srivastava via Phabricator via cfe-commits
Sunil_Srivastava added a comment.

A friendly ping.


https://reviews.llvm.org/D22057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-10-19 Thread Sunil Srivastava via cfe-commits
Sunil_Srivastava added a comment.

ping


https://reviews.llvm.org/D22057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-10-03 Thread Sunil Srivastava via cfe-commits
Sunil_Srivastava added a comment.

Ping


https://reviews.llvm.org/D22057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-09-14 Thread 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

Index: test/CodeGen/no-devirt.cpp
===
--- test/CodeGen/no-devirt.cpp
+++ test/CodeGen/no-devirt.cpp
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 %s -DUSEIT -triple %itanium_abi_triple -emit-llvm -o - |  FileCheck %s
+
+// Test with decls and template defs in pch, and just use in .cpp
+// RUN:  %clang_cc1 %s -DTMPL_DEF_IN_HEADER -triple %itanium_abi_triple -emit-pch -o %t
+// RUN:  %clang_cc1 %s -DTMPL_DEF_IN_HEADER -DUSEIT -triple %itanium_abi_triple -include-pch %t -emit-llvm -o - | FileCheck %s
+
+// Test with A in pch, and B and C in main
+// Test with just decls in pch, and template defs and use in .cpp
+// RUN:  %clang_cc1 %s -triple %itanium_abi_triple -emit-pch -o %t
+// RUN:  %clang_cc1 %s -DUSEIT -triple %itanium_abi_triple -include-pch %t -emit-llvm -o - | FileCheck %s
+
+#ifndef HEADER
+#define HEADER
+template < typename T, int N = 0 > class TmplWithArray {
+public:
+  virtual T& operator [] (int idx);
+  virtual T& func1 (int idx);
+  virtual T& func2 (int idx);
+  T ar[N+1];
+};
+struct Wrapper {
+  TmplWithArray data;
+  bool indexIt(int a) {
+if (a > 6) return data[a] ;  // Should not devirtualize
+if (a > 4) return data.func1(a); // Should devirtualize
+return data.func2(a);// Should devirtualize
+  }
+};
+
+#ifdef TMPL_DEF_IN_HEADER
+template  T& TmplWithArray::operator[](int idx) {
+  return ar[idx];
+}
+template  T& TmplWithArray::func1(int idx) {
+  return ar[idx];
+}
+#endif // TMPL_DEF_IN_HEADER
+#endif // HEADER
+
+#ifdef USEIT
+#ifndef TMPL_DEF_IN_HEADER
+template  T& TmplWithArray::operator[](int idx) {
+  return ar[idx];
+}
+template  T& TmplWithArray::func1(int idx) {
+  return ar[idx];
+}
+#endif // !TMPL_DEF_IN_HEADER
+extern Wrapper ew;
+bool stuff(int p)
+{
+  return ew.indexIt(p);
+}
+#endif
+
+// CHECK-NOT: call {{.*}} @_ZN13TmplWithArrayIbLi10EEixEi
+// CHECK-DAG: call {{.*}} @_ZN13TmplWithArrayIbLi10EE5func1Ei
+// CHECK-DAG: call {{.*}} @_ZN13TmplWithArrayIbLi10EE5func2Ei
+
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3580,6 +3580,7 @@
   // Try again at the end of the translation unit (at which point a
   // definition will be 

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

2016-09-08 Thread Arthur O'Dwyer via cfe-commits
Quuxplusone added inline comments.


Comment at: test/CodeGen/no-devirt.cpp:16
@@ +15,3 @@
+   if (a > 6) return data[a] ;  // Should not devirtualize
+   if (a > 4) return data.func1(a); // Should devirtualize
+   return data.func2(a);// Should devirtualize

Sunil_Srivastava wrote:
> Quuxplusone wrote:
> > This is a really dumb question from the peanut gallery, but, could you 
> > explain why these two cases (lines 15 and 16) should differ?  It really 
> > seems like both calls should be able to be devirtualized, because the 
> > compiler statically knows what they should call.
> > 
> > I think you mention the difference between lines 15 and 16 here:
> > 
> > > except, for some reason, when it is an operator and used with an operator 
> > > syntax
> > 
> > but you don't explain *why* the difference is desirable. Shouldn't we just 
> > fix that difference, then?
> > 
> > Is your first fix (
> > 
> > > The first fix will be in the front end to force the instantiation on 
> > > virtual calls that are potentially devirtualizable.
> > 
> > ) basically "fix the difference between lines 15 and 16", or is it talking 
> > about something else entirely?
> AFAICS, The two cases (line 15 and 16) should not differ.
> 
> The first fix will "fix the difference between line 15 and 16". I have the 
> change for that ready, but once we do that, there will be no way (known to 
> me) of testing the second "fix". Hence the second "fix" is being proposed for 
> commit before the first.
Okay, makes sense to me now. (As long as the comment on line 15 gets updated in 
the "first fix". I believe you'll be touching this test case again anyway in 
the "first fix", because the expected output will change.)
Question answered. :)


https://reviews.llvm.org/D22057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-09-07 Thread Sunil Srivastava via cfe-commits
Sunil_Srivastava added inline comments.


Comment at: test/CodeGen/no-devirt.cpp:16
@@ +15,3 @@
+   if (a > 6) return data[a] ;  // Should not devirtualize
+   if (a > 4) return data.func1(a); // Should devirtualize
+   return data.func2(a);// Should devirtualize

Quuxplusone wrote:
> This is a really dumb question from the peanut gallery, but, could you 
> explain why these two cases (lines 15 and 16) should differ?  It really seems 
> like both calls should be able to be devirtualized, because the compiler 
> statically knows what they should call.
> 
> I think you mention the difference between lines 15 and 16 here:
> 
> > except, for some reason, when it is an operator and used with an operator 
> > syntax
> 
> but you don't explain *why* the difference is desirable. Shouldn't we just 
> fix that difference, then?
> 
> Is your first fix (
> 
> > The first fix will be in the front end to force the instantiation on 
> > virtual calls that are potentially devirtualizable.
> 
> ) basically "fix the difference between lines 15 and 16", or is it talking 
> about something else entirely?
AFAICS, The two cases (line 15 and 16) should not differ.

The first fix will "fix the difference between line 15 and 16". I have the 
change for that ready, but once we do that, there will be no way (known to me) 
of testing the second "fix". Hence the second "fix" is being proposed for 
commit before the first.


https://reviews.llvm.org/D22057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-07-19 Thread Arthur O'Dwyer via cfe-commits
Quuxplusone added a subscriber: Quuxplusone.


Comment at: test/CodeGen/no-devirt.cpp:16
@@ +15,3 @@
+   if (a > 6) return data[a] ;  // Should not devirtualize
+   if (a > 4) return data.func1(a); // Should devirtualize
+   return data.func2(a);// Should devirtualize

This is a really dumb question from the peanut gallery, but, could you explain 
why these two cases (lines 15 and 16) should differ?  It really seems like both 
calls should be able to be devirtualized, because the compiler statically knows 
what they should call.

I think you mention the difference between lines 15 and 16 here:

> except, for some reason, when it is an operator and used with an operator 
> syntax

but you don't explain *why* the difference is desirable. Shouldn't we just fix 
that difference, then?

Is your first fix (

> The first fix will be in the front end to force the instantiation on virtual 
> calls that are potentially devirtualizable.

) basically "fix the difference between lines 15 and 16", or is it talking 
about something else entirely?


https://reviews.llvm.org/D22057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-07-18 Thread Richard Smith via cfe-commits
rsmith added inline comments.


Comment at: include/clang/AST/Decl.h:1602
@@ -1601,2 +1601,3 @@
   unsigned IsConstexpr : 1;
+  unsigned IsMarkedForPendingInstantiation : 1;
 

Drop the `MarkedFor` here. This flag *is* the mark from the point of view of a 
user of the AST.

This flag also seems very similar to the `Used` flag on the `Decl` base class, 
so a documentation comment might help. Something like "Indicates that an 
instantiation of this function has been required but may not yet have been 
performed. If the function has already been instantiated, the value of this 
flag is unspecified."

... though it would be nicer if the value of the flag were always correct, even 
in the case where the function has already been instantiated.


Comment at: include/clang/AST/Decl.h:1877-1878
@@ -1874,1 +1876,4 @@
 
+  /// \brief Whether this function has been put on the pending instatiations
+  /// list.
+  bool isMarkedForPendingInstantiation() const {

The fact that we have a separate list is an implementation detail of `Sema`; 
drop that from this comment.


Comment at: lib/CodeGen/CGClass.cpp:2884-2886
@@ -2879,3 +2883,5 @@
 if (const ValueDecl *VD = dyn_cast(ME->getMemberDecl()))
-  return VD->getType()->isRecordType();
+  return VD->getType()->isRecordType() &&
+  (MD->isMarkedForPendingInstantiation() ||
+   !MD->isImplicitlyInstantiable());
 

If the function is already defined (if we instantiated it already for some 
reason), we should be able to devirtualize calls to it.


Comment at: lib/Sema/Sema.cpp:683-685
@@ -682,2 +682,5 @@
   ExternalSource->ReadPendingInstantiations(Pending);
+  for (auto PII : Pending) 
+if (FunctionDecl *Func = dyn_cast(PII.first))
+  Func->setMarkedForPendingInstantiation();
   PendingInstantiations.insert(PendingInstantiations.begin(),

Please add a test that we handle this properly if instantiation is triggered in 
a PCH.


https://reviews.llvm.org/D22057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-07-18 Thread Sunil Srivastava via cfe-commits
Sunil_Srivastava added a comment.

Ping.


https://reviews.llvm.org/D22057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-07-06 Thread Sunil Srivastava via cfe-commits
Sunil_Srivastava created this revision.
Sunil_Srivastava added a reviewer: rsmith.
Sunil_Srivastava added a subscriber: cfe-commits.

This review is for a fix for PR 27895, but it requires some discussion. The 
bugzilla and the email exchange with Richard Smith in 
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160530/161107.html 
contain more details.

The basic problem: When the front end sees a call to a virtual function of a 
template class, it treats that as an odr use, therefore requiring the 
instantiation; except, for some reason, when it is an operator and used with an 
operator syntax. In those cases, if the call gets devirtualized later, it will 
generate a direct call to a function that has not been instantiated. Most of 
the time it will still work because someone else will instantiate it for some 
other reason, but if not, then we have converted a potentially dead code to a 
link time error of a missing definition.

As discussed in the email exchange, there are two alternative ways to fix the 
problem. Either will be sufficient, but we want to do both.

The first fix will be in the front end to force the instantiation on virtual 
calls that are potentially devirtualizable. For reasons described below I plan 
on submitting this in a later checkin.

The second fix, the matter of this review, is to avoid devirtualization under 
certain conditions. The conditions that I am choosing is to prevent 
devirtualization if the function could be implicitly instantiated but front end 
chose to not mark it for instantiation. I welcome any suggestions about 
improvement to this condition.

The reason I am planning on submitting them in this order, is that, once the 
first fix is checked in, there will be no way, known to me, to write a test for 
the second, devirtualization fix.

Now some discussion about the way I am testing the avoid-devirtualization 
condition: I have added a bit to FunctionDecl that I am setting whenever a 
function is put on the PendingInstantiations list. Then, at the time of 
devirtualization, if the target function isImplictlyInstantiable and does not 
have that bit set, I avoid the devirtualization. This change adds one bit to 
FunctionDecl, but the test is quite fast. However, it duplicates the 
information between this bit and the PendingInstantiationsList.

We have considered an alternate implementation which replaces the call (in 
CGClass.cpp) to 
isMarkedForPendingInstantiation with
   - explcitly-test-whether-it-is-on-the-PendingInstations-list ||
   - has-already-been-instantiated.

This alternate implementation does not add anything to FunctionDecl, avoids 
information duplication, but the test for its presence in the list will be 
expensive; it will require a search in the list. It will be done only for 
devirtualizable calls though, which is not that common.

A related point that I am not sure about is: Are there circumstances where a 
function on the PendingInstantiations list can fails to be instantiated for 
reasons other than errors? If so then this checkin is not perfect, though it is 
still better than nothing.

I am open to suggestion as to which of these implementations makes more sense.

Once this change has been approved and submitted, I will open another review 
for the first fix, which will force the instantiation of functions that are 
target of potentially devirtualizable calls.



http://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

Index: test/CodeGen/no-devirt.cpp
===
--- test/CodeGen/no-devirt.cpp
+++ test/CodeGen/no-devirt.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 %s -triple %itanium_abi_triple -emit-llvm -o - | FileCheck %s
+template < typename T, int N = 0 > class TmplWithArray {
+public:
+  virtual T& operator [] (int idx);
+  virtual T& func1 (int idx);
+  virtual T& func2 (int idx);
+  T ar[N+1];
+};
+class Wrapper {
+  TmplWithArray data;
+  bool indexIt(int a);
+};
+bool Wrapper::indexIt(int a)
+{
+   if (a > 6) return data[a] ;  // Should not devirtualize
+   if (a > 4) return data.func1(a); // Should devirtualize
+   return data.func2(a);// Should devirtualize
+}
+template  T& TmplWithArray::operator[](int idx) {
+  return ar[idx];
+}
+template  T& TmplWithArray::func1(int idx) {
+  return ar[idx];
+}
+// CHECK-NOT: call {{.*}} @_ZN13TmplWithArrayIbLi10EEixEi
+// CHECK-DAG: call {{.*}} @_ZN13TmplWithArrayIbLi10EE5func1Ei
+// CHECK-DAG: call {{.*}} @_ZN13TmplWithArrayIbLi10EE5func2Ei
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3531,6 +3531,7 @@
   // Postpone late parsed template instantiations.
   if