[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