Skip to content

Conversation

@andykaylor
Copy link
Collaborator

We have been using RecordLayoutAttr to "cache" data layout information calculated for records. Unfortunately, it wasn't actually caching the information, and because each call was calculating more information than it needed, it was doing extra work.

This replaces the previous implementation with a set of functions that compute only the information needed. Ideally, we would like to have a mechanism to properly cache this information, but until such a mechanism is implemented, these new functions should be a small step forward.

We have been using RecordLayoutAttr to "cache" data layout information
calculated for records. Unfortunately, it wasn't actually caching the
information, and because each call was calculating more information than
it needed, it was doing extra work.

This replaces the previous implementation with a set of functions that
compute only the information needed. Ideally, we would like to have a
mechanism to properly cache this information, but until such a mechanism
is implemented, these new functions should be a small step forward.
@xlauko
Copy link
Collaborator

xlauko commented Apr 18, 2025

LGTM

@bcardosolopes
Copy link
Member

Thanks guys!

@bcardosolopes bcardosolopes merged commit 390cf25 into llvm:main Apr 18, 2025
10 checks passed
terapines-osc-cir pushed a commit to Terapines/clangir that referenced this pull request Sep 2, 2025
We have been using RecordLayoutAttr to "cache" data layout information
calculated for records. Unfortunately, it wasn't actually caching the
information, and because each call was calculating more information than
it needed, it was doing extra work.

This replaces the previous implementation with a set of functions that
compute only the information needed. Ideally, we would like to have a
mechanism to properly cache this information, but until such a mechanism
is implemented, these new functions should be a small step forward.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants