[PATCH] D128333: [clang][flang] Disable defaulting to `-fpie` for LLVM Flang

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.


================
Comment at: flang/test/Driver/no-pie.f90:3
+
+!-------------
+! RUN COMMANDS
----------------
awarzynski wrote:
> MaskRay wrote:
> > The `! RUN COMMANDS` and `EXPECTED OUTPUT` comments seem rather redundant. I'd remove them.
> This style is quite common in this directory, see e.g. https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/emit-asm-aarch64.f90. I'm hoping that this makes these tests easier to follow for folks less familiar with LIT in general.
> 
> If that's OK, I'll leave this here (but I don't expect others to follow similar approach, IMO it's a matter of personal preference).
Since this is precedent, I assume this is fine. I will note here that this is contrary to the convention almost everywhere in llvm-project...


================
Comment at: flang/test/Driver/pic-flags.f90:6
+!-------------
+! RUN: %flang -### %s -target aarch64-linux-gnu 2>&1 | FileCheck %s --check-prefix=CHECK-NOPIE
+! RUN: %flang -### %s -target aarch64-linux-gnu -fno-pie 2>&1 | FileCheck %s --check-prefix=CHECK-NOPIE
----------------
In clang driver, `-target ` is a deprecated form (`Separate` style options are very uncommon in external command line utilities). The preferred spelling is `--target=`


================
Comment at: flang/test/Driver/pic-flags.f90:15
+! CHECK-NOPIE: "-fc1"
+! CHECk-NOPIE-NOT: fpic
+! CHECK-NOPIE: "{{.*}}ld"
----------------
fpic is not waterproof. The temporary object file name or the working directory may have `fpic` as a substring. Use `"-fpic"`


================
Comment at: flang/test/Driver/pic-flags.f90:20
+! CHECK-PIE: "-fc1"
+! TODO: Once Flang supports `-fpie`, it //should// use -fpic when invoking `flang -fc1`. Update the following line once `-fpie` is
+! available.
----------------
`! TODO:` may possibly be recognized a future FileCheck/lit as a stale check prefix. Personally I may use something like `!! TODO:`. I guess you are just following existing practice so this may be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128333