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

Arthur O'Dwyer via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 8 11:36:31 PDT 2016


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





More information about the cfe-commits mailing list