[PATCH] D61201: [LLD][ELF] Full support for -n (--nmagic) and -N (--omagic)

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 30 03:27:44 PDT 2019


grimar added a comment.

Generally looks good I think. Few more comments/suggestions from me.



================
Comment at: ELF/Config.h:167
   bool OptRemarksWithHotness;
+  bool Paged;
   bool PicThunk;
----------------
Should it be here? I think it is the place for the flags that
are named similar to a command line option. Since there is no `--paged` option,
should it be below, under the following comment?


```
(line 232)
 // The following config options do not directly correspond to any
 // particualr command line options.
```


================
Comment at: ELF/Driver.cpp:907
 
+  // -n (--nmagic) and -N (--omagic) disable Page Alignment of Sections.
+  Config->Paged = !(Config->Omagic ||
----------------
Not sure why `Page Alignment of Sections` are in upper case here..


================
Comment at: ELF/Driver.cpp:1122
+      Config->Static = true;
+      break;
     case OPT_whole_archive:
----------------
Should it be combined with `case OPT_Bstatic`?
That would be easier to find all the options that affect a particular flag, what is better I think.


================
Comment at: ELF/Options.td:222
+def nmagic: F<"nmagic">, MetaVarName<"<magic>">,
+  HelpText<"Do not page align sections, link against static libraries.">;
+
----------------
You should update `lld\docs\ld.lld.1` I think after adding an option.
The same for below ones.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61201/new/

https://reviews.llvm.org/D61201





More information about the llvm-commits mailing list