-
Notifications
You must be signed in to change notification settings - Fork 351
[lldb] Add support for multipayload enums lookup from DWARF #8171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: stable/20230725
Are you sure you want to change the base?
Conversation
430d71a to
12d3f5c
Compare
| } | ||
|
|
||
| uint32_t getContentsSizeInWords() const override { | ||
| return m_spare_bits_mask.size() / 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could benefit from a comment. Why 4; (is this dependent on ptrsize?) is this always divisible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using the same terminology as GenReflection for the function name here. A word in this case is 32 bits long so always divisible by 4.
| auto bit_offset = | ||
| child_die.GetAttributeValueAsUnsigned(llvm::dwarf::DW_AT_bit_offset, 0); | ||
|
|
||
| auto byte_offset = (bit_offset + 7) / 8; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use an llvm::align function from MatchExtras.h here?
| return child_die.Tag() == llvm::dwarf::DW_TAG_APPLE_spare_bits_mask; | ||
| }); | ||
|
|
||
| if (spare_bits_mask_die_it == child_die.children().end()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these error cases should probably all go to the dwarf log channel?
| attributes.ExtractFormValueAtIndex(0, form_value); | ||
|
|
||
| if (!form_value.IsValid()) { | ||
| if (auto *log = GetLog(LLDBLog::Types)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dwarf instead of Types? Or maybe both?
| // similar calculation when emitting this into metadata). | ||
| llvm::APInt bits(64, form_value.Unsigned()); | ||
| auto bitsInMask = bits.getActiveBits(); | ||
| uint32_t bytesInMask = (bitsInMask + 7) / 8; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use mathextras?
| auto bitsInMask = bits.getActiveBits(); | ||
| uint32_t bytesInMask = (bitsInMask + 7) / 8; | ||
| auto wordsInMask = (bytesInMask + 3) / 4; | ||
| bits = bits.zextOrTrunc(wordsInMask * 32); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this independent of ptrsize?
| if (isSingleWord()) | ||
| return U.VAL; | ||
| assert(getActiveBits() <= 64 && "Too many bits for uint64_t"); | ||
| /* assert(getActiveBits() <= 64 && "Too many bits for uint64_t"); */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to stay in.
No description provided.