evgeny777 added a comment.
This patch is to be rebased against D73094 <https://reviews.llvm.org/D73094>
================
Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:830
enum Kind {
+ Unknown, ///< Unknown (analysis not performed, don't lower)
Unsat, ///< Unsatisfiable type (i.e. no global has this type metadata)
----------------
I don't think such implicit conversion of Unsat to Unknown is good idea. I
guess problem will arise for legacy index files: for those ByteArray resolution
will become Unsat and so on. I suggest moving Unknown the end of the list and
bumping index version respectively (parseTypeIdSummaryRecord doesn't verify
TheKind).
================
Comment at: llvm/lib/Passes/PassBuilder.cpp:1380
+ // in ICP (which is performed earlier than this in the regular LTO pipeline).
+ MPM.addPass(LowerTypeTestsPass(nullptr, nullptr, true));
----------------
Can we get rid of two identical passes here (and in other places also)? For
instance
```
MPM.addPass(LowerTypeTestsPass(ExportSummary, nullptr, true));
```
can both lower type metadata and do final cleanup.
================
Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1678
+ // breaks any uses on assumes.
+ if (TypeIdMap.count(TypeId))
+ continue;
----------------
I don't think, I understand this.
It looks like that if (a) we have type.test in the module and (b) we don't have
vtable definition with corresponding type metadata in the same module then
we'll remove type.test/assume sequence before even getting to ICP. This seems
strange in the context of previous discussion because virtual function may be
called in different module from one where vtable is defined, like so:
```
struct A { virtual int foo() { return 42; } };
// calls pA->foo
extern int callFoo(A *pA);
int main() { A a; return callFoo(&a); }
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73242/new/
https://reviews.llvm.org/D73242
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
