ahatanak marked 2 inline comments as done.
ahatanak added inline comments.
================
Comment at: lib/Sema/SemaExpr.cpp:14715
+ if (Method->isVirtual() && !(Method->hasAttr<FinalAttr>() ||
+ Method->getParent()->hasAttr<FinalAttr>()))
OdrUse = false;
----------------
vsk wrote:
> Do you think it makes sense to eliminate all candidate virtual methods which
> can be devirtualized? If so, you could make
> "CanDevirtualizeMemberFunctionCall" a shared utility between Sema and
> CodeGen, and use it here. That function should give "the truth" about whether
> or not a call can be devirtualized.
I moved CanDevirtualizeMemberFunctionCall to DeclCXX.cpp and added overloaded
functions of canDevirtualizeCall and used one of them here. This caused changes
in the generated IR in two test cases (no-devirt.cpp and
vtable-available-externally.cpp). Both changes look correct to me.
================
Comment at: test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp:271
+ // Derived::operator() is not emitted, there will be a linker error.
+ (*ptr)();
+ }
----------------
vsk wrote:
> Have you looked into why "ptr->operator()();" compiles? We are either missing
> a devirtualization opportunity, or we have inconsistent logic for setting
> MightBeOdrUse for member calls. Either way, I think this patch is the right
> vehicle to address the issue.
"ptr->operator()();" creates a MemberExpr, which MarkMemberReferenced handles.
The difference between MarkMemberReferenced and MarkDeclRefReferenced is that
the former sets OdrUse to false when the method is pure while the latter does
so when the method is virtual. Prior to r174341, MarkDeclRefReferenced was
setting OdrUse to false for pure methods too (see r174242).
https://reviews.llvm.org/D34301
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
