[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

awarzynski added a comment. Victor, this is proving quite tricky to review. There's already been a lot of updates and many of them are summarized as either "code refactor" or "clean-up". Please reduce traffic/noise and use more descriptive summaries. Also, rather than adding new features in thi

[PATCH] D157837: [flang][driver] Update the visibility of Clang options in Flang

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG83a06997c69a: [flang][driver] Update the visibility of Clang options in Flang (authored by awarzynski). Changed prior to commit: https://reviews.l

[PATCH] D158289: [flang][driver] Partial revert of https://reviews.llvm.org/D157837

awarzynski created this revision. awarzynski added reviewers: DavidSpickett, kiranchandramohan. Herald added a reviewer: sscalpone. Herald added a project: All. awarzynski requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. This is a pa

[PATCH] D158289: [flang][driver] Partial revert of https://reviews.llvm.org/D157837

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG89053e495903: [flang][driver] Partial revert of D157837 (authored by awarzynski). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D158307: [flang][driver] Disable Clang options in Flang

awarzynski created this revision. Herald added a reviewer: sscalpone. Herald added a project: All. awarzynski requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Restore the desired setting that was reverted in https://reviews.llvm.o

[PATCH] D158309: [flang][driver] Mark -Wl as visible in Flang

awarzynski created this revision. awarzynski added a reviewer: kiranchandramohan. Herald added a reviewer: sscalpone. Herald added projects: Flang, All. awarzynski requested review of this revision. Herald added subscribers: cfe-commits, jdoerfert. Herald added a project: clang. Repository: rG L

[PATCH] D158309: [flang][driver] Mark -Wl as visible in Flang

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3758aed31a51: [flang][driver] Mark -Wl as visible in Flang (authored by awarzynski). Changed prior to commit: https://reviews.llvm.org/D158309?vs=

[PATCH] D158307: [flang][driver] Disable Clang options in Flang

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb16a7581ff8b: [flang][driver] Disable Clang options in Flang (authored by awarzynski). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D158507: [Flang][Driver] Add support for fomit-frame-pointer

awarzynski added a comment. > Temporary fix for unknown argument error '-fomit-frame-pointer' when running > flang tests I don't follow - there's quite a lot going on here. More than the summary suggests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D158612: [flang][driver] Ensure negative flags have the same visibility as positive

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. LGTM, ta! Comment at: clang/include/clang/Driver/Options.td:5314 NegFlag, - PosFlag, + PosFlag, BothFlags<[], [ClangOption], " the integrated assembler">>; --

[PATCH] D156524: [flang][nfc] Simplify option forwarding

awarzynski created this revision. awarzynski added a reviewer: kiranchandramohan. Herald added a reviewer: sscalpone. Herald added a subscriber: sunshaoce. Herald added projects: Flang, All. awarzynski requested review of this revision. Herald added subscribers: cfe-commits, jdoerfert, MaskRay. Her

[PATCH] D156524: [flang][nfc] Simplify option forwarding

awarzynski added a comment. Thanks for taking a look! Comment at: clang/lib/Driver/ToolChains/Flang.cpp:146-147 + + Args.AddAllArgs(CmdArgs, {options::OPT_flang_experimental_hlfir, +options::OPT_flang_experimental_polymorphism}); }

[PATCH] D156524: [flang][nfc] Simplify option forwarding

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe88ff8a79b8b: [flang][nfc] Simplify option forwarding (authored by awarzynski). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

awarzynski added a subscriber: everythingfunctional. awarzynski added a comment. In D125788#4556324 , @MaskRay wrote: > If you continue this work, consider landing this rename in multiple phases, > e.g. > > - Use lit substitutions instead of `flang-new`

[PATCH] D156320: [FLang] Add support for Rpass flag

awarzynski added a comment. Hey @victorkingi , thank you for working on this :) There's quite a lot going on here and I am thinking that it might be good to split this into a few patches? Also, please note that Flang's driver, unlike Clang, uses MLIR's coding style (use `camelCase` instead of `

[PATCH] D157410: [Flang][Driver] Enable Rpass and other R family options.

awarzynski accepted this revision. awarzynski added a comment. LG, with a small suggestion :) (in the spirit of cleaning up Options.td) Comment at: clang/include/clang/Driver/Options.td:819-831 +def Rpass_EQ : Joined<["-"], "Rpass=">, Group, Flags<[CC1Option, FlangOption, FC1O

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

awarzynski added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:173-174 +if (!result.Regex->isValid(regexError)) { + diags.Report(clang::diag::err_drv_optimization_remark_pattern) + << regexError << a->getAsString(args); + return fal

[PATCH] D157410: [Flang][Driver] Enable Rpass and other R family options.

awarzynski accepted this revision. awarzynski added a comment. Thanks! LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157410/new/ https://reviews.llvm.org/D157410 ___ cfe-commits mailing list cfe-com

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

awarzynski added inline comments. Comment at: flang/test/Driver/pic-flags.f90:3 -! 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

[PATCH] D154869: [Flang] [FlangRT] Implement FlangRT library as solution to Flang's runtime LLVM integration

awarzynski added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:934-936 + // Default to the /../lib and + // /../runtimes/runtimes-bins/lib directories. This works fine + // on the platforms that we have tested so far. We will probably have to

[PATCH] D154869: [Flang] [FlangRT] Implement FlangRT library as solution to Flang's runtime LLVM integration

awarzynski added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:934-936 + // Default to the /../lib and + // /../runtimes/runtimes-bins/lib directories. This works fine + // on the platforms that we have tested so far. We will probably have to

[PATCH] D158763: [flang][driver] Mark -L as visible in Flang

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. LGTM, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158763/new/ https://reviews.llvm.org/D158763 ___

[PATCH] D137995: [Flang][Driver] Handle target CPU and features

awarzynski added a comment. Thanks for implementing this! > Processes target cpu and features in the flang driver. Right now features are > only added for AArch64 because I only did basic testing on AArch64 but it > should generally work for others as well. X86 is a very popular target and we

[PATCH] D129156: Add -fpass-plugin option to Flang

awarzynski added a comment. In D129156#3949156 , @ro wrote: > This introduced a new failure on Solaris: > > FAIL: Flang :: Driver/pass-plugin-not-found.f90 > > Running the failing command manually shows: > > error: unable to load plugin 'X.Y': 'Could

[PATCH] D137995: [Flang][Driver] Handle target CPU and features

awarzynski added a comment. Thanks for all the updates @mnadeem! Mostly looks good. A few more nits, but nothing substantial :) In D137995#3958824 , @mnadeem wrote: > In D137995#3931005 , > @kiranchandramohan w

[PATCH] D137995: [Flang][Driver] Handle target CPU and features

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. LGTM, thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137995/new/ https://reviews.llvm.org/D137995 ___ cfe-commits mailin

[PATCH] D76078: [AArch64][SVE] Add a pass for SVE intrinsic optimisations

andwar added a comment. Thanks for the updates @kmclaughlin ! Would you mind adding a comment to clearly mark the negative tests? E.g. for `@reinterpret_reductions_1`? Maybe also a comment `why` a particular case is not optimised? You've already done that for some tests, but not all of them.

[PATCH] D76617: [SveEmitter] Fix encoding/decoding of SVETypeFlags

andwar added a comment. Cheers for the fix @sdesmalen ! Comment at: clang/include/clang/Basic/TargetBuiltins.h:187 -SVETypeFlags(uint64_t F) : Flags(F) {} -SVETypeFlags(EltType ET, bool IsUnsigned) : Flags(ET) {} +SVETypeFlags(uint64_t F) : Flags(F), EltTypeShift(

[PATCH] D76929: [AArch64][SVE] Add SVE intrinsic for LD1RQ

andwar added a comment. Cheers for working on this @kmclaughlin ! Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1407 case AArch64ISD::PTRUE: return "AArch64ISD::PTRUE"; + case AArch64ISD::LD1RQ: return "AArch64ISD::LD1RQ"; case AArch

[PATCH] D74912: [AArch64][SVE] Add SVE2 intrinsics for bit permutation & table lookup

andwar added a comment. Cheers for working on this @kmclaughlin! Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:2035 + +def int_aarch64_sve_bdep_x : AdvSIMD_2VectorArg_Intrinsic; +def int_aarch64_sve_bext_x : AdvSIMD_2VectorArg_Intrinsic; What does `_x`

[PATCH] D75160: [AArch64][SVE] Add SVE2 intrinsic for xar

andwar accepted this revision. andwar added a comment. This revision is now accepted and ready to land. LGTM, cheers for working on this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75160/new/ https://reviews.llvm.org/D75160 __

[PATCH] D76078: [AArch64][SVE] Add a pass for SVE intrinsic optimisations

andwar added a comment. Cheers for working on this @kmclaughlin ! Have you considered adding an interface for the new PM? You could check this for reference: https://reviews.llvm.org/rGd6de5f12d485a85504bc99d384a85634574a27e2 (also implements a FunctionPass). Comment at: llv

[PATCH] D76078: [AArch64][SVE] Add a pass for SVE intrinsic optimisations

andwar added a comment. > Have you considered adding an interface for the new PM? Please ignore that comment ^^^. I incorrectly assumed that the backend has also been ported to the new PM, but that's not the case. In particular, `llc` will only use the legacy PM anyway: https://github.com/llvm

[PATCH] D74550: [AArch64][SVE] Add SVE index intrinsic

andwar added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10923 +static SDValue LowerSVEIntrinsicIndex(SDNode *N, SelectionDAG &DAG) { + SDLoc dl(N); + SDValue Op1 = N->getOperand(1); [nit] This should be `DL`: https://llvm.org/

[PATCH] D146075: [flang][driver][openmp] Write MLIR for -save-temps

awarzynski added a comment. Please add tests ;) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146075/new/ https://reviews.llvm.org/D146075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D146075: [flang][driver][openmp] Write MLIR for -save-temps

awarzynski added a comment. In D146075#4196998 , @tschuett wrote: > What is the policy on trivial braces in the frontend? What do you mean by the frontend? Different people have different understanding of what "the frontend" is. I try to follow https:

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

awarzynski added inline comments. Comment at: flang/test/Driver/code-gen-rv64.f90:1 +! Test -emit-obj (RISC-V 64) + mamrami wrote: > Hi :) > It seems like the test fails: > https://buildkite.com/llvm-project/premerge-checks/builds/141302#0186e55e-199e-401d-ab9f-

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

awarzynski added inline comments. Herald added a subscriber: jplehr. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:128 + if (IsHostOffloadingAction) { +for (size_t i = 1; i < Inputs.size(); ++i) { + if (Inputs[i].getType() != types::TY_Nothing) What

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

awarzynski added inline comments. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:128 + if (IsHostOffloadingAction) { +for (size_t i = 1; i < Inputs.size(); ++i) { + if (Inputs[i].getType() != types::TY_Nothing) agozillon wrote: > awarzynski wrote: >

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. LGTM, thanks for working on this! Would be great for somebody with a bit more experience with offloading to OK this as well :) @tschuett or perhaps @jhuber6 ? Commen

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

awarzynski added inline comments. Comment at: flang/test/Driver/omp-frontend-forwarding.f90:1 +! REQUIRES: amdgpu-registered-target + jhuber6 wrote: > agozillon wrote: > > awarzynski wrote: > > > Given that you use `-###`, I think that this can be skipped (please

[PATCH] D146278: [flang] add -flang-experimental-hlfir flag to flang-new

awarzynski added a comment. Could you add some tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146278/new/ https://reviews.llvm.org/D146278 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D146278: [flang] add -flang-experimental-hlfir flag to flang-new

awarzynski added inline comments. Comment at: flang/test/Driver/mlir-pass-pipeline.f90:15-18 +! O2-NEXT: Canonicalizer +! ALL-NEXT: LowerHLFIRIntrinsics +! ALL-NEXT: BufferizeHLFIR +! ALL-NEXT: ConvertHLFIRtoFIR It looks like these passes are run unconditionally

[PATCH] D146278: [flang] add -flang-experimental-hlfir flag to flang-new

awarzynski added inline comments. Comment at: flang/test/Driver/mlir-pass-pipeline.f90:15-18 +! O2-NEXT: Canonicalizer +! ALL-NEXT: LowerHLFIRIntrinsics +! ALL-NEXT: BufferizeHLFIR +! ALL-NEXT: ConvertHLFIRtoFIR tblah wrote: > awarzynski wrote: > > It looks like

[PATCH] D146075: [flang][driver][openmp] Write MLIR for -save-temps

awarzynski added a comment. Thanks for the updates! I advice against using UPPER case in filenames. Please bear in mind that on Windows and MacOS filenames are case insensitive. It's just less hassle to stick to lower case. Comment at: flang/lib/Frontend/FrontendActions.cpp:

[PATCH] D146278: [flang] add -flang-experimental-hlfir flag to flang-new

awarzynski added inline comments. Comment at: flang/test/HLFIR/flang-experimental-hlfir-flag.f90:2 +! Test -flang-experimental-hlfir flag +! RUN: %flang_fc1 -flang-experimental-hlfir -emit-fir -o - %s | FileCheck %s + Could you also add a `RUN` line like this: ``

[PATCH] D145579: [Flang][AMDGPU][OpenMP] Save target features in OpenMP MLIR dialect

awarzynski added a comment. Really nice to see some shared code being elevated out of Clang into LLVM, thanks! I've only reviewed on the Flang driver changes. I will let the OpenMP experts to review the remaining bits. All in all looks good, I've only made some small suggestions. Thanks for w

[PATCH] D146278: [flang] add -flang-experimental-hlfir flag to flang-new

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: flang/test/HLFIR/flang-experimental-hlfir-flag.f90:19 +! CHECK-NEXT: } +! NO-HLFIR-NOT: hlfir. [ultra nit] I would separat

[PATCH] D145579: [Flang][AMDGPU] Add support for AMDGPU to Flang driver

awarzynski added a comment. A few more comments, but mostly nits. Btw, is this patch sufficient to generate code for AMDGPU? Or, put differently, what's the level of support atm? Comment at: clang/lib/Driver/ToolChains/Flang.cpp:107 break; + case llvm::Triple::r600: + c

[PATCH] D146075: [flang][driver][openmp] Write MLIR for -save-temps

awarzynski accepted this revision. awarzynski added a comment. This is a very nice patch, thanks for working on this! A few final nits, but feel free to ignore. LGTM Comment at: flang/lib/Frontend/FrontendActions.cpp:103 + std::error_code ec; + llvm::ToolOutputFile out(outDi

[PATCH] D146075: [flang][driver][openmp] Write MLIR for -save-temps

awarzynski accepted this revision. awarzynski added a comment. LGTM! Comment at: flang/test/Driver/save-temps.f90:14 ! CHECK-NEXT: "-o" "save-temps.o" ! CHECK-NEXT: "-o" "a.out" skatrak wrote: > awarzynski wrote: > > skatrak wrote: > > > awarzynski wrote: >

[PATCH] D146814: [Flang] Add debug flag to enable current debug information pass

awarzynski added a comment. Thanks for submitting this! Please add tests for `-g` and all variants of `-gline-tables-only`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146814/new/ https://reviews.llvm.org/D146814 ___

[PATCH] D145579: [Flang][AMDGPU] Add support for AMDGPU to Flang driver

awarzynski added inline comments. Comment at: flang/lib/Frontend/FrontendActions.cpp:139-142 + // Clang does not append all target features to the clang -cc1 invocation. + // Some AMDGPU features are passed implicitly by the Clang frontend. + // That's why we need to extract i

[PATCH] D146814: [Flang] Add debug flag to enable current debug information pass

awarzynski added a comment. What's the overall design goal here? 100% consistency with Clang? Could this be documented? Comment at: clang/include/clang/Driver/ToolChain.h:23 #include "llvm/ADT/StringRef.h" -#include "llvm/ADT/Triple.h" #include "llvm/Frontend/Debug/Options.h

[PATCH] D142347: [NFC][Clang] Move DebugOptions to llvm/Frontend for reuse in Flang

awarzynski accepted this revision. awarzynski added a comment. LGTM, thanks! As @kiranchandramohan points out, this is in line with the overall direction that we agreed on a while back. This change will very likely affect various downstream consumers - it would be helpful to advertise this with

[PATCH] D145579: [Clang][Flang][AMDGPU] Add support for AMDGPU to Flang driver

awarzynski added a comment. Thanks for the updates, mostly looks good. Just a couple of extra questions about the test coverage. Comment at: flang/lib/Frontend/FrontendActions.cpp:139-142 + // Clang does not append all target features to the clang -cc1 invocation. + // Some

[PATCH] D146814: [Flang] Add debug flag to enable current debug information pass

awarzynski added a comment. Thanks for the updates! A few more pointers, but nothing major. Btw, are there any tests that check for debug info in the compiled files? For example: ! RUN: flang -g1 -S %s | FileCheck -check-prefixes=DEBUG-INFO-PRESENT ! RUN: flang -g0 -S %s | FileCheck -check-

[PATCH] D146814: [Flang] Add debug flag to enable current debug information pass

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. LGTM, thanks for working on this! (please fix the name in CommonArgs.h before landing this) It would be great to see a test that checks for debug info in the generated FIR/MLIR/LLVM-I

[PATCH] D146814: [Flang] Add debug flag to enable current debug information pass

awarzynski added a comment. Thanks for the explanation! Still looking good apart from the function names ;) Comment at: clang/lib/Driver/ToolChains/CommonArgs.h:111 +llvm::codegenoptions::DebugInfoKind +DebugLevelToInfoKind(const llvm::opt::Arg &A); + awarzynsk

[PATCH] D145579: [Clang][Flang][AMDGPU] Add support for AMDGPU to Flang driver

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. Thanks for implementing this, LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145579/new/ https://reviews.llvm.org/D145579 ___

[PATCH] D146814: [Flang] Add debug flag to enable current debug information pass

awarzynski added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.h:111 +llvm::codegenoptions::DebugInfoKind +DebugLevelToInfoKind(const llvm::opt::Arg &A); + SBallantyne wrote: > awarzynski wrote: > > awarzynski wrote: > > > CamelCase or camel

[PATCH] D143704: [Flang] Part one of Feature List action

awarzynski added a comment. In D143704#4150680 , @jdoerfert wrote: > can we have a test? +1 Comment at: flang/examples/FeatureList/FeatureList.cpp:34 + +struct ASTVisitor { +private: There is no AST in Flang - could y

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

awarzynski added inline comments. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:121-125 + if (IsOpenMPDevice) { +// -fopenmp-is-device is passed along to tell the frontend that it is +// generating code for a device, so that only the relevant declarations are +//

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. > Thank you very much @awarzynski for suggesting them that was a great help. I'm happy that I could help :) The driver logic LGTM, but please wait for either @jdoerfert and/or @kiranch

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

awarzynski added inline comments. Comment at: clang/test/Driver/flang/flang-omp.f90:1 +! Check that flang -fc1 is invoked when in --driver-mode=flang +! and the relevant openmp and openmp offload flags are utilised agozillon wrote: > agozillon wrote: > > awarzyn

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

awarzynski added inline comments. Comment at: clang/test/Driver/flang/flang-omp.f90:1 +! Check that flang -fc1 is invoked when in --driver-mode=flang +! and the relevant openmp and openmp offload flags are utilised agozillon wrote: > awarzynski wrote: > > agozil

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

awarzynski added inline comments. Comment at: clang/test/Driver/flang/flang-omp.f90:1 +! Check that flang -fc1 is invoked when in --driver-mode=flang +! and the relevant openmp and openmp offload flags are utilised agozillon wrote: > awarzynski wrote: > > agozil

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

awarzynski accepted this revision. awarzynski added a comment. In D144864#4175001 , @agozillon wrote: > And then provided @awarzynski is happy with the current state of the patch LGTM, thanks for the updates and for working on this! Repository: rG LL

[PATCH] D145845: [Flang] Allow compile *.f03, *.f08 file

awarzynski added a comment. Thanks! In D145845#4186590 , @clementval wrote: > Thanks. Can you add tests in `flang/test/Driver`? Hm, this is a `clangDriver` change rather than anything Flang specific 🤔 . Btw, how does https://github.com/llvm/llvm-proje

[PATCH] D145845: [Flang] Allow compile *.f03, *.f08 file

awarzynski added a comment. In D145845#4186608 , @clementval wrote: > In D145845#4186607 , @awarzynski > wrote: > >> Thanks! >> >> In D145845#4186590 , @clementval >> w

[PATCH] D145845: [Flang] Allow compile *.f03, *.f08 file

awarzynski added a comment. I'd move these tests to flang/test/Driver/supported-suffices. That would help documenting what these tests actual check for. Also, one could be tempted to test for other suffices and then it would be nice to keep them in one place. Ta! Comment at:

[PATCH] D145845: [Flang] Allow compile *.f03, *.f08 file

awarzynski added inline comments. Comment at: flang/test/Driver/supported-suffices/f08-suffix.f08:4 +! CHECK: "{{.*}}flang-new" "-fc1" {{.*}} "/tmp/{{.*}}.o" +! CHECK: "{{.*}}ld" {{.*}} "/tmp/{{.*}}.o" +program f08 This line will not appear without `-flang-experi

[PATCH] D145845: [Flang] Allow compile *.f03, *.f08 file

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145845/new/ https://reviews.llvm.org/D145845 __

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

awarzynski added a comment. Why does "flang/test/Driver/target-features.f90" list all RISC-V features? Why not use https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/target-cpu-features.f90 instead? Comment at: flang/test/Driver/target-features.f90:1 +! RUN: %

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

awarzynski added inline comments. Comment at: flang/test/Driver/target-features.f90:1 +! RUN: %flang --target=riscv64-linux-gnu --target=riscv64 -c %s -### 2>&1 \ +! RUN: | FileCheck %s -check-prefix=CHECK-RV64 jrtc27 wrote: > awarzynski wrote: > > What happens i

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

awarzynski added a comment. Herald added a subscriber: jobnoorman. > Fix the issue of .o file generated by Flang with Flags info is 0x0 under > RISC-V. TBH, I don't see how this is addressed in this patch. If that's something that this patch is intending to fix, then there should be a regressio

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

awarzynski added inline comments. Comment at: flang/test/Driver/code-gen-rv64.f90:12 + +! CHECK: Flags: 0x5, RVC, double-float ABI +end program For those of us less familiar with RISC-V - could you explain what's significant about this line? For example, [[ htt

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

awarzynski added inline comments. Comment at: flang/test/Driver/code-gen-rv64.f90:2 +! Test -emit-obj (RISC-V 64) + +! RUN: rm -f %t.o Missing `REQUIRES: ` - this test won't work unless the RISC-V backend is available. Comment at: flang/test/

[PATCH] D145815: [Flang][Driver] Add support for fopenmp-is-device and fembed-offload-object to Flang ToolChain

awarzynski added inline comments. Comment at: flang/test/Driver/omp-frontend-forwarding.f90:1 +! REQUIRES: amdgpu-registered-target + Given that you use `-###`, I think that this can be skipped (please double check). Comment at: flang/test/Dri

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

awarzynski added inline comments. Comment at: flang/test/Driver/target-cpu-features.f90:1 -! REQUIRES: aarch64-registered-target, x86-registered-target +! REQUIRES: aarch64-registered-target, x86-registered-target, riscv-registered-target vzakhari wrote: > Can

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

awarzynski added inline comments. Comment at: flang/test/Driver/code-gen-rv64.f90:12 + +! CHECK: Flags: 0x5, RVC, double-float ABI +end program sunshaoce wrote: > awarzynski wrote: > > awarzynski wrote: > > > For those of us less familiar with RISC-V - could you

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

awarzynski added inline comments. Comment at: flang/test/Driver/code-gen-rv64.f90:7 +! RUN: %flang_fc1 -triple riscv64-unknown-linux-gnu \ +! RUN: -target-feature +d -target-feature +c -emit-obj %s -o %t.o +! RUN: llvm-readelf -h %t.o | FileCheck %s jrtc27 wrot

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. LGTM, thanks for contributing! Please wait for the other reviewers to confirm that they are happy with these changes. Comment at: flang/test/Driver/target-cpu-featu

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

awarzynski added inline comments. Comment at: flang/test/Driver/target-cpu-features-invalid.f90:13 +! CHECK-INVALID-CPU: 'supercpu' is not a recognized processor for this target (ignoring processor) +! CHECK-INVALID-FEATURE: '+superspeed' is not a recognized feature for this ta

[PATCH] D153281: [flang] add -flang-experimental-polymorphism flag to flang-new

awarzynski accepted this revision. awarzynski added a comment. LGTM, thanks David! Could you also add a test in https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/frontend-forwarding.f90? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D153379: Remove -flang-experimental-exec flag

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. This is in line with the conclusions of https://discourse.llvm.org/t/proposal-rename-flang-new-to-flang/: > We should remove the need for the -flang-experimental-exec flag. There’s no

[PATCH] D145264: [OpenMP][MLIR][Flang][Driver][bbc] Lower and apply Module FlagsAttr

awarzynski added a comment. The driver plumbing looks good to me. I will defer reviewing the OpenMP changes to experts. Thanks for working on this! Comment at: clang/include/clang/Driver/Options.td:2692 def fno_openmp_assume_teams_oversubscription : Flag<["-"], "fno-openmp-a

[PATCH] D145264: [OpenMP][MLIR][Flang][Driver][bbc] Lower and apply Module FlagsAttr

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. @agozillon Thanks for the updates, LGTM! Please wait for somebody to review the OpenMP changes before merging. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D141307: Add -f[no-]loop-versioning option

awarzynski added a comment. Mats, thanks for working on this! Just a few minor suggestions from me. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:55 +static bool shouldLoopVersion(const ArgList &Args) { + if (Arg *A = Args.getLastArg(options::OPT_Ofast, options::OPT_O, -

[PATCH] D147941: [Flang][Driver][OpenMP] Enable flags for filtering of offloading passes in flang

awarzynski added a comment. Could you add tests that demonstrate what these options actually do? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147941/new/ https://reviews.llvm.org/D147941 ___ cfe-commits

[PATCH] D147941: [Flang][Driver][OpenMP] Enable flags for filtering of offloading passes in flang

awarzynski added a comment. In D147941#4255461 , @skatrak wrote: > In D147941#4255458 , @awarzynski > wrote: > >> Could you add tests that demonstrate what these options actually do? > > Thank you for the quick r

[PATCH] D147941: [Flang][Driver][OpenMP] Enable options for selecting offloading phases in flang

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. LGTM, thanks for addressing my comments. I'd wait a day before landing this - just in case other reviewers have comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D141307: Add -f[no-]loop-versioning option

awarzynski accepted this revision. awarzynski added a comment. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141307/new/ https://reviews.llvm.org/D141307 ___ cfe-commits mailing list cfe-co

[PATCH] D151088: [flang][hlfir] Separate -emit-fir and -emit-hlfir for flang-new

awarzynski added a comment. Thanks Tom, mostly makes sense, but I suggest the following: - `EmitMLIR` --> `EmitFIR` (nfc, just clarifying the name), - `EmitHLFIRToFIR` --> `EmitHLFIR` (functional change). More comments inline. Comment at: flang/include/flang/Frontend/Frontend

[PATCH] D151088: [flang][hlfir] Separate -emit-fir and -emit-hlfir for flang-new

awarzynski added inline comments. Comment at: flang/lib/Frontend/FrontendActions.cpp:651 +// Lower using HLFIR then run the FIR to HLFIR pipeline +void CodeGenAction::lowerHLFIRToFIR() { + assert(mlirModule && "The MLIR module has not been generated yet."); tbla

[PATCH] D151088: [flang][hlfir] Separate -emit-fir and -emit-hlfir for flang-new

awarzynski accepted this revision. awarzynski added a comment. Thanks for the updates, LGTM! Comment at: flang/test/HLFIR/flang-experimental-hlfir-flag.f90:7-12 +! | Action | -flang-experimental-hlfir? | Result | +! | === | ===

[PATCH] D143478: [RFC][Flang][driver] Try to support `flang -fc1as`

awarzynski added a comment. Thanks for working on this! Before diving any deeper - Does LLVM Flang require `flang-new -fc1as`? - If yes, why can't Flang and Clang share it? For better visibility, I suggest raising these questions on Discourse . As this could potenti

[PATCH] D143493: [flang][driver] Add support for -include flag in flang -fc1

awarzynski added a comment. In what cases would this flag be used in practice? I've scanned Clang and couldn't find any answers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143493/new/ https://reviews.llvm.org/D143493 __

[PATCH] D143493: [flang][driver] Add support for -include flag in flang -fc1

awarzynski added a comment. In D143493#4110272 , @skatrak wrote: > It is introduced to the arguments list in `Clang::AddPreprocessingOptions` in > certain cases to add the OpenMP wrapper "__clang_openmp_device_functions.h" > to the list of includes for

[PATCH] D143301: Emit warning for unsupported gfortran flags

awarzynski added a comment. In D143301#4126682 , @jdoerfert wrote: > Split this into two patches/reviews. +1 > I think the -W stuff can go in, it has tests and is reasonable. I'd like for us to rely on a flag from Options.td for this instead. Something

<    1   2   3   4   5   6   7   >