[PATCH] D152159: [AppleAccelTable] Refactor iterator class

Felipe de Azevedo Piovezan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 5 06:46:06 PDT 2023


fdeazeve created this revision.
Herald added a subscriber: hiraditya.
Herald added a project: All.
fdeazeve requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

The current implementation of the AppleAcceleratorTable::Entry is problematic
for a few reasons:

1. It is heavyweight. Iterators should be cheap, but the current implementation

tracks 3 different integer values, one "Entry" object, and one pointer to the
actual accelerator table. Most of this information is redundant and can be
removed.

2. It performs "memory reads" outside of the dereference operation. This

violates the usual expectations of iterators, whereby we don't access anything
so long as we don't dereference  the iterator.

3. It doesn't commit to tracking _one_ thing only. It tries to track both an

"index" into a list of HashData entries and a pointer in a blob of data. For
this reason, it allows for multiple "invalid" states, keeps redundant
information around and is difficult to understand.

4. It couples the interpretation of the data with the iterator increment. As

such, if the *interpretation* fails, the iterator will keep on producing garbage
values without ever indicating so to consumers.

The problem this iterator is trying to solve is simple: we have a blob of data
containing many "HashData" entries and we want to iterate over them. As such,
this commit makes the iterator only track a pointer over that data, and it
decouples the iterator increments from the interpretation of this blob of data.

We maintain the already existing assumption that failures never happen, but now
make it explicit with an assert.

Depends on D152158 <https://reviews.llvm.org/D152158>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152159

Files:
  llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
  llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D152159.528415.patch
Type: text/x-patch
Size: 6547 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230605/50781a46/attachment.bin>


More information about the llvm-commits mailing list