[PATCH] D99353: [driver] Make `clang` warn rather then error on `flang` options

awarzynski abandoned this revision. awarzynski added a comment. Herald added a subscriber: MaskRay. Herald added a project: All. With the imminent switch to GitHub and no new comments in over 2 years, I feel that it's time to abandon this change. Feel free to re-use this in the future :)

[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] 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] 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

[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

[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:

[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

[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

[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] 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

[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:

[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

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

awarzynski updated this revision to Diff 550668. awarzynski added a comment. Rebase on top of main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157837/new/ https://reviews.llvm.org/D157837 Files: clang/include/clang/Driver/Options.td

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

awarzynski added a comment. hey @victorkingi , I am still unsure about parsing these remarks options in two places: - CompilerInvocation.cpp - ExecuteCompilerInvocation.cpp I think that it is important to clarify the relations between the two. In particular, it's normally the job of

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

awarzynski added a comment. Some high level comments: - The logic in `parseCodeGenArgs` in CompilerInvocation.cpp is a bit complex and quite specialized - could you move it to a dedicated method? - In a fair few places this patch make references to "diagnostic flags" or "diagnostic options".

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. While quite extensive, I find the overall logic in this patch very well structured and executed in a very clean manner. It removes a lot of ambiguity, makes the overall design much

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

awarzynski added a comment. Thanks for the updates - this is looking really good! A few more suggestions and then I'll scan the whole thing again (sorry, there's been quite a lot of code going back and forth). Comment at: flang/lib/Frontend/CompilerInvocation.cpp:1024 +

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

awarzynski updated this revision to Diff 550047. awarzynski added a comment. Add missing `TargetSpecific` flag to the definition of `mcpu_EQ`, remove redundant `TODO` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157837/new/

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

awarzynski marked 2 inline comments as done. awarzynski added a comment. Thank you all for reviewing! In D157837#4584387 , @bogner wrote: > I can't speak to which flags should be present in flang-new or not That's determined by what's tested/used in

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

awarzynski added a comment. In D157151#4582441 , @bogner wrote: > This is a little bit complicated by the fact that the Option library is > already partially extracted out of clang (and used for other drivers, like > lld and lldb). The "Default"

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

awarzynski created this revision. awarzynski added a reviewer: bogner. Herald added a reviewer: sscalpone. Herald added projects: Flang, All. awarzynski requested review of this revision. Herald added subscribers: cfe-commits, wangpc, jplehr, sstefan1, jdoerfert, MaskRay. Herald added a reviewer:

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

awarzynski added a comment. Thanks for trimming this, it's much easier to review! A few more suggestions, but nothing major. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:227-263 + bool needLocTracking = false; + + if (!opts.OptRecordFile.empty()) +

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

awarzynski added a comment. I've tested this locally and can confirm that the behavior of `clang` and `flang-new` has been preserved (as in, these changes won't be visible to the end users). Nice! I think that it would be good to replace `Default` with e.g. - `Clang`, or - `ClangDriver`, or -

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

awarzynski added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:156-158 +/// Parse a remark command line argument. It may be missing, disabled/enabled by +/// '-R[no-]group' or specified with a regular expression by '-Rgroup=regexp'. +/// On top of that,

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

awarzynski added a comment. Hey @bogner , I've only skimmed through so far and it's looking great! That Include/Exclude API was not fun to use. What you are proposing here takes Options.td to a much a better place - this is a much needed and long overdue refactor. There's quite a bit of churn

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

awarzynski added a comment. Thanks for the updates, more comments inline. Also: > The remarks printed miss carets. Why and can you share an example? > The parseOptimizationRemark, addDiagnosticArgs and printDiagnosticOptions > functions created are identical to the ones used in Clang. In

[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

[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

[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,

[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] 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] 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

[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

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.

[PATCH] D155452: [Flang] Add support for fsave-optimization-record

awarzynski accepted this revision. awarzynski added a comment. LGTM, thanks for the updates! (please address my comment in "frontend-forwarding.f90" before landing this) Comment at: flang/test/Driver/frontend-forwarding.f90:26 +! RUN: %flang -### %s 2>&1 \ +! RUN:

[PATCH] D155452: [Flang] Add support for fsave-optimization-record

awarzynski added a comment. Thanks for working on this, LG! I've left a few minor comments inline. Comment at: clang/include/clang/Driver/Options.td:6449-6455 +def opt_record_file : Separate<["-"], "opt-record-file">, Flags<[FC1Option, CC1Option]>, + HelpText<"File name 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] 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] 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

[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] 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

[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] 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.");

[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:

[PATCH] D150354: [OpenMP][MLIR][Flang][bbc][Driver] Add fopenmp-version and generate corresponding MLIR attribute

awarzynski added a comment. All in all LGTM, but I'm not sure whether Flang should be defaulting to OpenMP 5.0. AFAIK, that's not supported yet. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:34 + Args.AddAllArgs(CmdArgs, {options::OPT_ffixed_form, +

[PATCH] D134821: [flang][driver] Allow main program to be in an archive

awarzynski added a comment. In D134821#4317371 , @peixin wrote: >> @peixin , wdyt? > > @awarzynski Hi Andrzej, sorry for the late reply. I am distracted by several > internal projects and other things in my life recently (just came back from >

[PATCH] D134821: [flang][driver] Allow main program to be in an archive

awarzynski added a comment. I've just reverted this patch - for context please see https://reviews.llvm.org/D149429. @sunshaoce This was committed without acknowledging @ekieri 's contribution, so didn't follow the official

[PATCH] D134821: [flang][driver] Allow main program to be in an archive

awarzynski accepted this revision. awarzynski added a comment. In D134821#4301701 , @sunshaoce wrote: > Any further suggestions? I am happy for you to land this, thanks! LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D134821: [flang][driver] Allow main program to be in an archive

awarzynski added a comment. Thanks for all the great effort, @ekieri ! @sunshaoce , mostly makes sense, just a few small suggestions. @peixin , wdyt? Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:601-603 +// A Fortran main program will be lowered to a function named

[PATCH] D148038: [Flang][OpenMP][Driver][MLIR] Port fopenmp-host-ir-file-path flag and add MLIR module attribute to proliferate to OpenMP IR lowering

awarzynski added a comment. In D148038#4292549 , @agozillon wrote: > In D148038#4292262 , @awarzynski > wrote: > >> LGTM, thanks for addressing my comments! > > Thank you for your time and the great review

[PATCH] D148038: [Flang][OpenMP][Driver][MLIR] Port fopenmp-host-ir-file-path flag and add MLIR module attribute to proliferate to OpenMP IR lowering

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

[PATCH] D148038: [Flang][OpenMP][Driver][MLIR] Port fopenmp-host-ir-file-path flag and add MLIR module attribute to proliferate to OpenMP IR lowering

awarzynski added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:730 +res.getLangOpts().OMPHostIRFile = arg->getValue(); +if (!llvm::sys::fs::exists(res.getLangOpts().OMPHostIRFile)) +

[PATCH] D148038: [Flang][OpenMP][Driver][MLIR] Port fopenmp-host-ir-file-path flag and add MLIR module attribute to proliferate to OpenMP IR lowering

awarzynski added inline comments. Comment at: clang/include/clang/Driver/Options.td:6558 HelpText<"Path to the IR file produced by the frontend for the host.">, - Flags<[CC1Option, NoDriverOption]>; + Flags<[CC1Option, FC1Option, NoDriverOption]>; With

[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

[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

[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

[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 ___

[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 ) { + if (Arg *A = Args.getLastArg(options::OPT_Ofast, options::OPT_O,

[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

[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<["-"],

[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 ); + SBallantyne wrote: > awarzynski wrote: > > awarzynski wrote: > > > CamelCase or

[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 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 ); + awarzynski

[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

[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

[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] 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

[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

[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

[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] 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] 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

[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: +

[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

[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

[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] 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:

[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] 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 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

[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

[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 ?

[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 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)

[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: >

[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

[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

[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

[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:

[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

[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/target-cpu-features.f90:1 -! REQUIRES: aarch64-registered-target, x86-registered-target +! REQUIRES: aarch64-registered-target, x86-registered-target, riscv-registered-target vzakhari wrote: >

[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:

[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:

[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, [[

[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

[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

[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] 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] 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

  1   2   3   4   5   6   7   >