[AsmPrinter][Dwarf5][nfc] Remove template from AccelTable class

[felipepiovezan]

This template is no longer used.

[felipepiovezan]

Removed inapplicable comment

[ayermolo]

Thanks for the PR. I just came back from PTO. Will take a look this week. 👍

[felipepiovezan]

Thanks for the PR. I just came back from PTO. Will take a look this week. 👍

Ditto! :)

[dwblaikie]

SGTM - if you're feeling so inclined, I wouldn't mind a quick summary of why this did exist and what made it no longer needed - but it's not super important if it's no longer needed in any case.

[felipepiovezan]

SGTM - if you're feeling so inclined, I wouldn't mind a quick summary of why this did exist and what made it no longer needed - but it's not super important if it's no longer needed in any case.

This was introduced in 2018 by @JDevlieghere in 98062cb
I believe the idea is that, for dsymutil, Entry data was stored by using a pointer to a DIE instead of as individual bits of information (DIE offset, etc...).

Recently, these different implementations were combined by using a std::variant.

[ayermolo]

Thanks for updating. Sorry didn't get to it earlier.

[dwblaikie]

Recently, these different implementations were combined by using a std::variant.

@ayermolo for posterity, could you explain/point to an existing explanation for that? (Sorry, I know I reviewed the code, but it's a lot to try to page back in, etc - perhaps we talked about it already elsewhere so a pointer to taht discussion would be good if we had one)

[ayermolo]

Recently, these different implementations were combined by using a std::variant.

@ayermolo for posterity, could you explain/point to an existing explanation for that? (Sorry, I know I reviewed the code, but it's a lot to try to page back in, etc - perhaps we talked about it already elsewhere so a pointer to taht discussion would be good if we had one)

This was done in #69399. Which was a pre-cursor to the .debug_names + types implementation for monolithic DWARF.

Previously for .debug_names we carried CU DIE pointer until the end when .debug_names is written out. Initially I changed so that the same happens to TU DIEs, but that lead to memory overhead increase when input is llvm IR. So I changed implementation back to write out TUs as they are being finalized. This means that an TableData needs to carry finalized offset for TU, and DIE for CU (until the latter is normalized so that when we write out .debug_names all TableData instances have just offset). Thus I changed the data structure to have both using the std::variant. In that PR we also discussed using std::variant vs union.

[dwblaikie]

Recently, these different implementations were combined by using a std::variant.

@ayermolo for posterity, could you explain/point to an existing explanation for that? (Sorry, I know I reviewed the code, but it's a lot to try to page back in, etc - perhaps we talked about it already elsewhere so a pointer to taht discussion would be good if we had one)

This was done in #69399. Which was a pre-cursor to the .debug_names + types implementation for monolithic DWARF.

Previously for .debug_names we carried CU DIE pointer until the end when .debug_names is written out. Initially I changed so that the same happens to TU DIEs, but that lead to memory overhead increase when input is llvm IR. So I changed implementation back to write out TUs as they are being finalized. This means that an TableData needs to carry finalized offset for TU, and DIE for CU (until the latter is normalized so that when we write out .debug_names all TableData instances have just offset). Thus I changed the data structure to have both using the std::variant. In that PR we also discussed using std::variant vs union.

nod thanks for the refresher - so since LLVM's normal emisison path used the DIE, and I guess DWARFLinker or something else needed the offset? And then since LLVM needed both anyway, the other codepath was changed to use that representation and so we only needed the one representation...

[ayermolo]

Recently, these different implementations were combined by using a std::variant.

@ayermolo for posterity, could you explain/point to an existing explanation for that? (Sorry, I know I reviewed the code, but it's a lot to try to page back in, etc - perhaps we talked about it already elsewhere so a pointer to taht discussion would be good if we had one)

This was done in #69399. Which was a pre-cursor to the .debug_names + types implementation for monolithic DWARF.
Previously for .debug_names we carried CU DIE pointer until the end when .debug_names is written out. Initially I changed so that the same happens to TU DIEs, but that lead to memory overhead increase when input is llvm IR. So I changed implementation back to write out TUs as they are being finalized. This means that an TableData needs to carry finalized offset for TU, and DIE for CU (until the latter is normalized so that when we write out .debug_names all TableData instances have just offset). Thus I changed the data structure to have both using the std::variant. In that PR we also discussed using std::variant vs union.

nod thanks for the refresher - so since LLVM's normal emisison path used the DIE, and I guess DWARFLinker or something else needed the offset? And then since LLVM needed both anyway, the other codepath was changed to use that representation and so we only needed the one representation...

Yep DWARF5AccelTableStaticData was used in DWARFLInker and DWARFLinkerParalle.