[PATCH] D11859: Generating vptr assume loads

hfinkel@anl.gov via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 13 16:41:03 PDT 2015


hfinkel added a subscriber: hfinkel.

================
Comment at: lib/CodeGen/CGClass.cpp:1832
@@ +1831,3 @@
+  // Generate vtable assumptions if we are calling dynamic class ctor
+  // and we are not in another ctor.
+  if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
----------------
I think this comment should be a little more verbose. How about this:

  // Generate vtable assumptions if we are calling dynamic-class's ctor, except when doing so as part of a derived class's ctor's base-class initialization. Doing so in this latter case would be useless, because the vtable is about to be overwritten by the derived class's vtable.


================
Comment at: test/CodeGenCXX/vtable-assume-load.cpp:32
@@ +31,3 @@
+// CHECK1-LABEL: define void @_ZN5test14fooAEv()
+// CHECK1: %cmp.vtables = icmp eq i8** %vtable, getelementptr inbounds ([3 x i8*], [3 x i8*]* @_ZTVN5test11AE, i64 0, i64 2)
+// CHECK1: call void @llvm.assume(i1 %cmp.vtables)
----------------
Will these IR values still have names even when compiling an -Asserts (NDEBUG) build? If not, you'll need to have better pattern matching.

Also, please match the load instructions that calculate %vtable (and the call to the ctor so that we know that the call to the assumption comes after it).

I think it would be nicer to put the checks for each function adjacent to the function definitions themselves. So put these CHECK lines right next to fooA().


================
Comment at: test/CodeGenCXX/vtable-assume-load.cpp:38
@@ +37,3 @@
+// CHECK1: call void @_ZN5test11BC1Ev(%"struct.test1::B"* %b)
+// CHECK1: %vtable = load i8**, i8*** %1, !tbaa !5
+// CHECK1: %cmp.vtables = icmp eq i8** %vtable, getelementptr inbounds ([3 x i8*], [3 x i8*]* @_ZTVN5test11BE, i64 0, i64 2)
----------------
Don't match the TBAA metadata (leaving off everything after the comma before !tbaa should be fine).



http://reviews.llvm.org/D11859





More information about the cfe-commits mailing list