Depends on D29854
Details
Diff Detail
- Build Status
Buildable 3939 Build 3939: arc lint + arc unit
Event Timeline
llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp | ||
---|---|---|
392 | Document new functions | |
765 | There are now a bunch of places where this array is being cleared. Suggest putting this in a helper method called something like CSInfo.markDevirtualized() - it will make it more explicit as to what is going on. | |
794 | I was trying to prove to myself that Res != nullptr when isExported(), but I am having trouble doing so. isExported() will be true when the CSInfo has SummaryTypeCheckedLoadUsers or SummaryHasTypeTestAssumeUsers - these are set up when (Action == PassSummaryAction::Export). But Res will only be non-null when (Action == PassSummaryAction::Export && isa<MDString>(S.first.TypeID)). Is there a case where we could reach here when Action == PassSummaryAction::Export but !isa<MDString>(S.first.TypeID)? | |
llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll | ||
33 | Why check these - is there a chance they could be modified if the optimization goes wrong? | |
llvm/test/Transforms/WholeProgramDevirt/import.ll | ||
40 | Add comment on why the optimization can't/shouldn't be applied here. |
- Address review comments
llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp | ||
---|---|---|
794 | I think isExported() can only possibly be true for type IDs of type MDString because we only add MDStrings to MetadataByGUID (see http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/WholeProgramDevirt.cpp#1204) and we only ever set SummaryTypeCheckedLoadUsers or SummaryHasTypeTestAssumeUsers on members of MetadataByGUID. | |
llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll | ||
33 | Yes, as with D29846 I want to make sure we aren't taking the general VCP code path. | |
llvm/test/Transforms/WholeProgramDevirt/import.ll | ||
40 | I refreshed this change past r296948 which added a comment at the top of this function. |
Document new functions