[PATCH] D131533: [Flang][Driver] Enable PIC in the frontend

Usman Nadeem via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 22 11:09:56 PDT 2022


mnadeem marked 3 inline comments as done.
mnadeem added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:6320-6325
+def pic_level : Separate<["-"], "pic-level">,
+  HelpText<"Value for __PIC__">,
+  MarshallingInfoInt<LangOpts<"PICLevel">>;
+def pic_is_pie : Flag<["-"], "pic-is-pie">,
+  HelpText<"File is for a position independent executable">,
+  MarshallingInfoFlag<LangOpts<"PIE">>;
----------------
awarzynski wrote:
> MaskRay wrote:
> > awarzynski wrote:
> > > awarzynski wrote:
> > > > These are code-gen options to me. While originally located under "Language Options", I think that it would make more sense to move them near "CodeGen Options" instead (e.g. near `mrelocation_model`). @MaskRay any thoughts?
> > > Turns out that in Clang these options are indeed [[ https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/LangOptions.def#L199 | LangOptions ]]. That's a bit confusing to me, but oh well.
> > clang/lib/Frontend/InitPreprocessor.cpp defines `__PIC__`.. IIUC the file does not know CodeGenOptions, so LangOptions isn't a bad choice.
> I think that we all agree that these options should remain somewhere withing the "Language Options" block, i.e. below:
> 
> ```
> //===----------------------------------------------------------------------===//
> // Language Options
> //===----------------------------------------------------------------------===//
> ```
> 
> As previously, you can use a `let` statement there:
> ```
> let Flags = [CC1Option, FC1Option, NoDriverOption] in {
> def pic_level : Separate<["-"], "pic-level">,
>   HelpText<"Value for __PIC__">,
>   MarshallingInfoInt<LangOpts<"PICLevel">>;
> def pic_is_pie : Flag<["-"], "pic-is-pie">,
>   HelpText<"File is for a position independent executable">,
>   MarshallingInfoFlag<LangOpts<"PIE">>;
> 
> } // let Flags = [CC1Option, FC1Option, NoDriverOption]
> ```
Sorry, missed that. Fixed now.


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

https://reviews.llvm.org/D131533



More information about the cfe-commits mailing list