[PATCH] D25073: [DebugInfo]: preparation to implement DW_AT_alignment

Victor Leschuk via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 10 13:31:54 PDT 2016


vleschuk marked an inline comment as done.
vleschuk added inline comments.


================
Comment at: include/llvm/IR/DebugInfoMetadata.h:1829
   unsigned Line;
+  uint64_t AlignInBits;
 
----------------
aprantl wrote:
> Sorry for noticing this so late:
> Does this really need to be a 64bit integer, or would a 32-bit unsigned be sufficient?
> A 32-bit unsigned could immediately follow the previous unsigned field und thus save 8 bytes.
SizeInBits and OffsetInBits in DebugInfoMetadata scope are 64bit. If we change this to 32bit for efficiency I'd suggest to make it as separate commit and change all of them.


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:1735
+  //   In this case Record size will be 10.
+  // We don't want to put any fake values into Record[1] and Record[9], we
+  // just encode the information that we have alignment value in Record[0],
----------------
aprantl wrote:
> It is more useful to future readers to explain what we did, rather than explain what alternatives we rejected ands why.
> I would just enumerate the 3(?) different formats of this record and how to differentiate them.
Done. Updated comment.


https://reviews.llvm.org/D25073





More information about the llvm-commits mailing list