[PATCH] D28040: Don't store the NULL DIEs in the DIE array in the DWARFUnit.

[Paul Robinson via Phabricator via llvm-commits]

probinson added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugInfoEntry.h:38
+  /// If non-zero this DIE is the last child in a sibling chain.
+  uint32_t IsLastChild : 1;
 
----------------
clayborg wrote:
> dblaike commented in email:
> 
> > I think you might be misreading the DWARF (& it looks like the DWARF is a bit confusing, as well).
> > 
> > For starters - in both your examples, the byte_size of the struct is 4 bytes. So it seems storage is shared, or at least that using the same type doesn't improve/reduce the size of the struct - so on that basis I'd probably suggest using the bool type.
> > 
> > Beyond that, though. The AT_data_member_location is +3, as you say - not +4, so it is still using the top (or bottom - the counting here is a bit weird imho, but seems to work out) byte of the previous 4 byte value.
> > 
> > Also - we have /lots/ of similar mixed type bitfields in llvm, I think (have a grep around - it looks like it to me, at least) - so if this isn't doing the right thing I'd be /really/ surprised. 
> 
> I did misread the DWARF, it is correct. I will switch this to bool.
No, please leave it as uint32_t.  MSVC will do what you thought the DWARF said, and make the struct bigger unnecessarily.

@dblaikie yes we have mixed-type bitfields in llvm, but we also have places that deliberately use the same type when space savings matter.  I see commits to change bitfield base types now and again for exactly that reason.


https://reviews.llvm.org/D28040