[PATCH] D20331: [llvm-mc] - Teach llvm-mc to generate compressed debug sections in zlib style.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Wed May 18 08:22:46 PDT 2016


grimar added inline comments.

================
Comment at: lib/MC/ELFObjectWriter.cpp:576
@@ -575,4 +575,3 @@
   auto &Sec = cast<MCSectionELF>(Sym->getSection());
-  unsigned Flags = Sec.getFlags();
-  if (Flags & ELF::SHF_MERGE) {
+  if (Sec.Flags & ELF::SHF_MERGE) {
     if (C != 0)
----------------
davide wrote:
> davide wrote:
> > grimar wrote:
> > > davide wrote:
> > > > You can change that separately, if you really like. It's unrelated, and makes the patch slightly less readable.
> > > Please see my comment below.
> > I can review them at the same time but they should be committed separately.
> I saw your comment. There's no need to change at the same time the access control and remove the getter. Leave it private, and leave the getter to keep the patch small.  You can do that in a separate commit.
Sorry but that means I need to intoduce setter to change Flags field ?
```
// Set the compressed flag. That is zlib style.
  Section.Flags |= ELF::SHF_COMPRESSED;

```
So do I correctly understand that you want me to create setter for this patch and remove it and getter in following commit ? what is the point for that ? May be let me just make Flags public at first ?

================
Comment at: lib/MC/ELFObjectWriter.cpp:1024
@@ +1023,3 @@
+    // Write Elf64_Chdr header.
+    write((ELF::Elf64_Word)ELF::ELFCOMPRESS_ZLIB);
+    write((ELF::Elf64_Word)0); // ch_reserved field.
----------------
davide wrote:
> static_cast<> instead of C-style cast?
will do. quick search shows that both way are used, but I would agree here.


http://reviews.llvm.org/D20331





More information about the llvm-commits mailing list