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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
