Fix parsing out-of-order ValueInfos #73239
Conversation
llvm/test/Assembler/thinlto-typid.ll
Outdated
|
|
||
| ^0 = module: (path: "thinlto-typeid.ll", hash: (0, 0, 0, 0, 0)) | ||
| ^1 = gv: (name: "_ZTVN3FooE", summaries: (variable: (module: ^0, flags: (linkage: internal, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 1, canAutoHide: 0), varFlags: (readonly: 1, writeonly: 0, constant: 1, vcall_visibility: 0), vTableFuncs: ((virtFunc: ^3, offset: 16))))) | ||
| ^4 = gv: (name: "_ZTSN3FooE", summaries: (variable: (module: ^0, flags: (linkage: linkonce_odr, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), varFlags: (readonly: 0, writeonly: 0, constant: 1)))) |
There was a problem hiding this comment.
Is this actually a bug on the assembly printer side? I.e. this should be ^2, I don't think it makes sense to print two summaries with the same identifier.
|
I am not really familiar with devirtualization part of the summary but is the real bug that there should not be two Maybe we should correctly error out instead of assertion in this case. |
This might be a bug (actually I think the problem is in one-guid-one-id approach in asm printer). However it seems that asm parser also contains a bug, because it can't really handle out of order ids. |
I just sent #73997 for the fix to the AsmWriter. I agree, though, that it would be good to make the parser more flexible to handle out of order ids. I think your change also handles that case (i.e. non-repeated but simply out of order ids). Can you update this patch to reflect that it is just for that issue, and the test case as well? |
eleviant
force-pushed
the
fix-summary-asm-parser
branch
from
f0947ef
to
a6937d9
Compare
|
Addressed comments |
| @@ -0,0 +1,24 @@ | |||
| ; RUN: llvm-as %s -o - | llvm-dis -o - | FileCheck %s | |||
|
|
|||
| ; CHECK: ^1 = gv: | |||
There was a problem hiding this comment.
Ideally the test would also ensure that the references between the renumbered entries are correct.
| @@ -0,0 +1,24 @@ | |||
| ; RUN: llvm-as %s -o - | llvm-dis -o - | FileCheck %s | |||
There was a problem hiding this comment.
Add short description of what is being tested at top
C++ class type info has two entries in summary index: gv and typeidCompatibleVTable, both sharing the same GUID. This causes value numbers being non-sequential
eleviant
force-pushed
the
fix-summary-asm-parser
branch
from
a6937d9
to
2c14e4c
Compare
|
Addressed comments |
C++ class type info has two entries in summary index: gv and typeidCompatibleVTable, both sharing the same GUID. This causes value numbers being non-sequential