[PATCH] D96344: [flang][driver] Add options for -fdefault* and -flarge-sizes

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. I tested this with `FLANG_BUILD_NEW_DRIVER` set to`On` and `Off` - all works fine. It's been a tricky patch @arnamoy10 , thank you for all the effort! I believe that you've addressed a

[PATCH] D96344: [flang][driver] Add options for -fdefault* and -flarge-sizes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGab971c29a56a: [flang][driver] Add options for -fdefault* and -flarge-sizes (authored by arnamoy10, committed by awarzynski). Changed prior to commit

[PATCH] D96875: [flang][driver] Add -fdebug-module-writer option

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/D96875/new/ https://reviews.llvm.org/D96875 ___ cfe-commits mailing

[PATCH] D97457: [flang][driver] Add `-fdebug-dump-parsing-log`

awarzynski updated this revision to Diff 328970. awarzynski added a comment. Generalise the test so that it's not architecture specific I've just realised that the test was failing in Phabricator's CI. It turns out it's an X86-only failure (i.e. the test works fine on my AArch64 dev machine). T

[PATCH] D98191: [flang][driver] Add support for `-fdebug-dump-symbols-sources`

awarzynski created this revision. Herald added a reviewer: sscalpone. Herald added subscribers: jansvoboda11, dang. awarzynski requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Note that the original spelling in `f18` is `-fget-symbols-source

[PATCH] D97080: [flang][driver] Add -fintrinsic-modules-path option

awarzynski added a comment. In D97080#2586289 , @arnamoy10 wrote: > Also, not sure what to set as the default directory, as gfortran uses a > specific installation location. We probably should use a location relative to the `flang-new` binary, i.e. `/..

[PATCH] D97457: [flang][driver] Add `-fdebug-dump-parsing-log`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG523d7bc6f427: [flang][driver] Add `-fdebug-dump-parsing-log` (authored by awarzynski). Changed prior to commit: https://reviews.llvm.org/D97457?vs

[PATCH] D96771: [OpenCL] Add distinct file extension for C++ for OpenCL

awarzynski accepted this revision. awarzynski added a comment. Thank you for addressing my comments and for working on this! I've left one small suggestion, but that's a [nit]. Comment at: clang/test/Driver/cxx_for_opencl.cppcl:1 +// RUN: %clang %s -Xclang -verify -fsyntax-onl

[PATCH] D97080: [flang][driver] Add -fintrinsic-modules-path option

awarzynski added a comment. In D97080#2613628 , @awarzynski wrote: > **Question**: What are the semantics for this flag in `gfortran`? Is the path > specified with `-fintrinsics-module-path` _prepended_ or _appended_ to the > default search path? I beli

[PATCH] D97080: [flang][driver] Add -fintrinsic-modules-path option

awarzynski added a comment. Thank you for updating this @arnamoy10 ! Comment at: flang/lib/Frontend/CompilerInvocation.cpp:26 #include "llvm/Support/raw_ostream.h" + +#include "llvm/Support/FileSystem.h" Not needed Comment at: flang/lib/Fron

[PATCH] D97080: [flang][driver] Add -fintrinsic-modules-path option

awarzynski added a comment. I tried reproducing the CI failures reported at the top (in particular `intrinsic_module_path.f90`), but haven't had much luck. @arnamoy10 - how about you? Could you `clang-format` this patch when uploading the next diff? Thank you! Comment at: fl

[PATCH] D97119: [flang][driver] Add options for -std=f2018

awarzynski added inline comments. Comment at: clang/include/clang/Driver/Options.td:3535-3536 MarshallingInfoFlag>; -def pedantic : Flag<["-", "--"], "pedantic">, Group, Flags<[CC1Option]>, - MarshallingInfoFlag>; +def pedantic : Flag<["-", "--"], "pedantic">, Group, Flags<

[PATCH] D136080: [flang] Add -ffp-contract option processing

awarzynski added a comment. > The omission of the fast-honor-pragmas argument from the compiler driver is > deliberate. Where is it omitted? > I suspect the CI failure on windows is unrelated to my code I agree. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:165 + // Fl

[PATCH] D136080: [flang] Add -ffp-contract option processing

awarzynski added inline comments. Comment at: flang/test/Driver/driver-help-hidden.f90:34 ! CHECK-NEXT: Use as character line width in fixed mode +! CHECK-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs): fast (fuses across statements disregarding pragmas) | on (only fuses i

[PATCH] D136080: [flang] Add -ffp-contract option processing

awarzynski added inline comments. Comment at: clang/include/clang/Driver/Options.td:1926 + " Default is 'fast' for CUDA, 'fast-honor-pragmas' for HIP, 'off' for flang, and 'on' otherwise.">, + HelpText<"Form fused FP ops (e.g. FMAs): fast | off (clang, flang), on | fast-honor

[PATCH] D136080: [flang] Add -ffp-contract option processing

awarzynski added a comment. Added Clang reviewers who touched the definition of `--fp-contract` most recently. Mostly for visibility, but lets give them at least a couple of days to take a look at the changes in Options.td. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136080/new/ htt

[PATCH] D136080: [flang] Add -ffp-contract option processing

awarzynski added a comment. Thanks for all the updates, Tom! I have a few more suggestions. From the summary: > implement these pragmas Could you explain what pragmas you are referring to here? (i.e. Clang pragmas for C and C++ + link) > gfortran uses "fast" by default For our future self, c

[PATCH] D136080: [flang] Add -ffp-contract option processing

awarzynski accepted this revision. awarzynski added a comment. LGTM, thanks for implementing this! Comment at: clang/lib/Driver/ToolChains/Flang.cpp:98-99 +} else + // Clang's "fast-honor-pragmas" option is not supported because it is + // non-standard and pragmas

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

awarzynski added a comment. Left a few more comments and also added Diana as a reviewer. Would be good to get an extra pair of eyes on this :) Comment at: clang/include/clang/Driver/Options.td:6320-6325 +def pic_level : Separate<["-"], "pic-level">, + HelpText<"Value for __PI

[PATCH] D131808: [clang,flang] Add missing options fsyntax-only in help

awarzynski added a comment. This is primarily a Clang change, so added some Clang reviewers. I will review shortly - thanks for taking this on! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131808/new/ https://reviews.llvm.org/D131808 ___

[PATCH] D131808: [clang,flang] Add missing options fsyntax-only in help

awarzynski added a comment. Thanks, mostly makes sense! https://github.com/llvm/llvm-project/issues/57033 mentions `-O{n}` as well :) Could you fix that too? More comments inline. CI is still failing :( Are you able to re-produce that? (I'm traveling atm, so can't check). Co

[PATCH] D131808: [clang,flang] Add missing options fsyntax-only in help

awarzynski accepted this revision. awarzynski added a comment. LGTM, thanks for the fix! You may want to update to title to better reflect the contents (e.g. “Add help text for -fsyntax-only”). While the fact that it fixes https://github.com/llvm/llvm-project/issues/57033 motivated this change,

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

awarzynski accepted this revision. awarzynski added a comment. Thanks for all the updates and for working on this! I'm not an expert in the semantics of `-fpie`/`-fpic`/`-mrelocation-model`, but this basically replicates the logic in Clang and I am not aware of any good reasons for Flang to div

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

awarzynski added inline comments. Comment at: flang/include/flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h:7 +// +//===--===// + Could you document what these are? And what are they used

[PATCH] D132513: [lit] Implement DEFINE and REDEFINE directives

awarzynski added a comment. I've skimmed through - these fixes make sense to me. Thank you for the quick summary! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132513/new/ https://reviews.llvm.org/D132513 _

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

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. Great stuff @ekieri , thanks for doing this! You may want add a note in the summary that you've updated the docs as well. This is consistent with what we discussed in https://github.

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

awarzynski added a comment. I suspect that this fails when running `ninja check-flang`, right? Most likely `Bye` needs to be added as a dependency for Flang tests, something akin to this

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

awarzynski added a comment. In D129156#3847396 , @tarunprabhu wrote: > Added `examples` to `REQUIRES` in `test/Driver/pass-plugin.f90. Thanks for the update! > I still cannot reproduce the build failure on my end. @MatsPetersson tested > this patch an

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

awarzynski added a comment. In D129156#3848704 , @clementval wrote: >>> I still cannot reproduce the build failure on my end. @MatsPetersson tested >>> this patch and the tests passed. >> >> @MatsPetersson & @clementval , could you share you build comma

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

awarzynski added a comment. In @clementval 's CMake invocation `LLVM_BUILD_EXAMPLES` is not set, which means that the examples (e.g. the `Bye` plugin) are not built. Adding `! REQUIRES: examples` to the test should fix the failure in this case. I did verify locally. However, the pre-commit CI

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

awarzynski added a comment. @tarunprabhu Could you confirm that with the build command above "pass-plugin.f90" is failing for you? It is for me. In order to fix that, you will have to add this

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

awarzynski added a comment. In D129156#3852553 , @tarunprabhu wrote: > In D129156#3851843 , @awarzynski > wrote: > >> @tarunprabhu Could you confirm that with the build command above >> "pass-plugin.f90" is fai

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

awarzynski accepted this revision. awarzynski added a comment. The driver looks good, thanks for all the effort! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130513/new/ https://reviews.llvm.org/D130513 ___ cfe-commits mailing list cfe-commi

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

awarzynski added a comment. In D129156#3853045 , @tarunprabhu wrote: > The tests still passed. No, it wasn't run. We need to understand why before proceeding. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129156/new/ https://reviews.llvm.org/

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

awarzynski abandoned this revision. awarzynski added a comment. In D125788#3622274 , @clementval wrote: > There are open discussion so wait for other to confirm or not. I was under the impression that we did discuss this extensively in our community ca

[PATCH] D130078: [flang][nfc] Rename `AddOtherOptions` as `ForwardOptions`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG174e954e370e: [flang][nfc] Rename `AddOtherOptions` as `ForwardOptions` (authored by awarzynski). Changed prior to commit: https://reviews.llvm.or

[PATCH] D136080: [flang] Add -ffp-contract option processing

awarzynski added a comment. Thanks for implementing this, @tblah! Two high level questions/requests: - are you confident that we will need LangOptions.def? - can you upload a patch with full context? (some details can be found here: https://llvm.org/docs/Phabricator.html#requesting-a-review-via

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to enable/disable ExternalNameConversionPass

awarzynski added a comment. Hi @madanial , thanks for posting this! > This patch adds user option -funderscoring/-fnounderscoring which behaves > similar to the gfortran option be enabling/disabling the > ExternalNameConversionPass I don't quite understand what this option is for and it's hard

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to enable/disable ExternalNameConversionPass

awarzynski added a comment. In D140795#4031392 , @kkwli0 wrote: > The purpose of this option is to control the trailing underscore being > appended to external names (e.g. procedure names, common block names). The > option in gfortran is documented in

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

awarzynski added inline comments. Comment at: flang/test/CMakeLists.txt:65 ) +if (NOT WIN32) + list(APPEND FLANG_TEST_DEPENDS Bye) IIUC, `Bye` is only needed when plugins are enabled. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129156/new/ https://

[PATCH] D137329: [flang] Add -f[no-]associative-math and -mreassociate

awarzynski added a comment. In D137329#3909082 , @clementval wrote: > Wouldn't it be good to have a RFC for all these options and what they will do > in Flang instead of just adding them all? Or did I miss the RFC? +1 Repository: rG LLVM Github Mon

[PATCH] D137329: [flang] Add -f[no-]associative-math and -mreassociate

awarzynski added a comment. In D137329#3910249 , @kiranchandramohan wrote: > In D137329#3909943 , @awarzynski > wrote: > >> In D137329#3909082 , @clementval >> wrote: >

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

awarzynski added a comment. Please be aware of https://reviews.llvm.org/D137673 - you may need to rebase if it lands before this patch. Please just go ahead and merge, but please keep `WIN32` in the final version of "flang/test/CMakeLists.txt" (see my comment inline). LGTM

[PATCH] D140972: [flang] Add -fstack-arrays flag

awarzynski accepted this revision. awarzynski added a comment. Thanks for the updates, LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140972/new/ https://reviews.llvm.org/D140972 ___ cfe-commits mai

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

awarzynski added a comment. Driver changes LGTM, thanks! I will defer to others for changes in other areas. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140795/new/ https://reviews.llvm.org/D140795 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D140972: [flang] Add -fstack-arrays flag

awarzynski added inline comments. Herald added a subscriber: sunshaoce. Comment at: clang/include/clang/Driver/Options.td:5056-5060 +def fstack_arrays : Flag<["-"], "fstack-arrays">, Group, + HelpText<"Attempt to allocate array temporaries on the stack, no matter their size">;

[PATCH] D140972: [flang] Add -fstack-arrays flag

awarzynski added inline comments. Comment at: flang/include/flang/Tools/CLOptions.inc:159 inline void createDefaultFIROptimizerPassPipeline( -mlir::PassManager &pm, llvm::OptimizationLevel optLevel = defaultOptLevel) { +mlir::PassManager &pm, bool stackArrays = false, l

[PATCH] D140972: [flang] Add -fstack-arrays flag

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: flang/test/Transforms/stack-arrays.f90:22-29 +! LLVM-IR: array_value_copy_simple +! LLVM-IR-NOT: malloc +! LLVM-IR-NOT: free +! LLVM-IR: all

[PATCH] D140972: [flang] Add -fstack-arrays flag

awarzynski added inline comments. Comment at: clang/include/clang/Driver/Options.td:485-486 +// Works like BoolOption except without specifying a KeyPathAndMacro, as these +// would refer to non-existant members of clang data structures +multiclass BoolFlangOnlyOptionhttps://re

[PATCH] D126291: [flang][Driver] Update link job on windows

awarzynski added a comment. In D126291#3577133 , @rovka wrote: > I had the same idea about switching the tests to using target triples instead > of having separate files for it, but initially I had some issues getting that > to work properly. Could you

[PATCH] D126291: [flang][Driver] Update link job on windows

awarzynski added a comment. 🤔 linker-flags.f90 is failing on Windows in the pre-commit CI. Not sure why - seems fine on Debian. Comment at: flang/test/Driver/linker-flags.f90:28 +! GNU-SAME: -lFortranDecimal +! WITHLM-SAME: -lm + Does `-SAME` makese sense here

[PATCH] D126291: [flang][Driver] Update link job on windows

awarzynski added inline comments. Comment at: flang/test/Driver/linker-flags.f90:28 + +! GNU-WITHOUTLM-LABEL: "{{.*}}ld" +! GNU-WITHOUTLM-SAME: "[[object_file]]" I think that GNU in this case might be a bit misleading. These linker invocations are defined in a

[PATCH] D126291: [flang][Driver] Update link job on windows

awarzynski added inline comments. Comment at: flang/test/Driver/linker-flags.f90:51 +! MSVC-NOT: libcmt +! MSVC-NOT: oldnames +! MSVC-SAME: "[[object_file]]" rovka wrote: > mmuetzel wrote: > > Lines 50-51 seem to be duplicates of lines 44-45. Is this intentional?

[PATCH] D126291: [flang][Driver] Update link job on windows

awarzynski added inline comments. Comment at: flang/test/Driver/linker-flags.f90:51 +! MSVC-NOT: libcmt +! MSVC-NOT: oldnames +! MSVC-SAME: "[[object_file]]" rovka wrote: > awarzynski wrote: > > rovka wrote: > > > mmuetzel wrote: > > > > Lines 50-51 seem to be du

[PATCH] D126291: [flang][Driver] Update link job on windows

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. LGTM, many thanks for this non trivial effort! :) I've left a few nits, feel free to ignore! @mstorsjo , are you also OK with this change? [nit] "This is exactly what we do for Linux/D

[PATCH] D128043: [flang][driver] Add support for `-O{0|1|2|3}`

awarzynski created this revision. awarzynski added reviewers: rovka, kiranchandramohan, schweitz, peixin. Herald added subscribers: bzcheeseman, rriddle, mgorny. Herald added a reviewer: sscalpone. Herald added projects: Flang, All. awarzynski requested review of this revision. Herald added subscri

[PATCH] D127207: [flang][driver] Fix support for `-x`

awarzynski added a comment. In D127207#3593665 , @sunho wrote: > Hi! I'm not exactly sure what's going on. But, seems like build is failing > here? https://lab.llvm.org/buildbot/#/builders/177/builds/5571 Thanks for flagging this up! I am also rather co

[PATCH] D128043: [flang][driver] Add support for `-O{0|1|2|3}`

awarzynski added a comment. In D128043#3596096 , @peixin wrote: > The CI failed. Thanks - I didn't notice any failures related to this change. Did I miss anything? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

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

awarzynski created this revision. awarzynski added reviewers: rovka, MaskRay, schweitz, Leporacanthicus. Herald added subscribers: jsji, StephenFan, pengfei, kristof.beyls. Herald added a reviewer: sscalpone. Herald added projects: Flang, All. awarzynski requested review of this revision. Herald ad

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

awarzynski added a comment. In D128333#3601299 , @kiranchandramohan wrote: > Is the longer-term plan to support this in Flang as well? I don't see why not. AFAIK, the switch in Clang took a while and happened gradually - so we probably shouldn't rush t

[PATCH] D128043: [flang][driver] Add support for `-O{0|1|2|3}`

awarzynski added a comment. Thanks for taking a look, @peixin! Just to clarify - I'm not really looking into `-Os`, `-Ofast` or `-Oz` at the moment. But I'm always happy to review driver patches :) Comment at: clang/include/clang/Driver/Options.td:732 +def O_flag : Flag<["-"]

[PATCH] D128043: [flang][driver] Add support for `-O{0|1|2|3}`

awarzynski updated this revision to Diff 438995. awarzynski marked 3 inline comments as done. awarzynski added a comment. Address comments from Peixin and Diana Main change - removed ENUM_CODEGENOPT and VALUE_CODEGENOPT from CodeGenOptions.dev. Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D128043: [flang][driver] Add support for `-O{0|1|2|3}`

awarzynski added inline comments. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:133 + CmdArgs.push_back("-O3"); + TC.getDriver().Diag(diag::warn_O4_is_O3); +} else { peixin wrote: > Nit: I have committed D126164, and you can rebase and use D dir

[PATCH] D128043: [flang][driver] Add support for `-O{0|1|2|3}`

awarzynski updated this revision to Diff 439086. awarzynski added a comment. Use `D` instead of `TC.getDriver()` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128043/new/ https://reviews.llvm.org/D128043 Files: clang/include/clang/Driver/Options

[PATCH] D128043: [flang][driver] Add support for `-O{0|1|2|3}`

awarzynski added a comment. In D128043#3604037 , @rovka wrote: > I just realized I haven't pestered you enough about testing :) I did feel like I was missing something here, haha! Updates arriving shortly! Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D128043: [flang][driver] Add support for `-O{0|1|2|3}`

awarzynski updated this revision to Diff 439285. awarzynski added a comment. Add tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128043/new/ https://reviews.llvm.org/D128043 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/Too

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

awarzynski added a comment. Thank you all for your comments and apologies for going radio silent - I was away for a few days. I've identified 2 threads emerging from your comments: **1. flang vs flang-new** @sscalpone, if I understand correctly, you are suggesting that any tool named `flang` i

[PATCH] D126291: [flang][Driver] Update link job on windows

awarzynski added a comment. Thanks for adding this, Diana! :) Overall, makes sense to me. I have a couple of questions/points. In D126291#3547023 , @rovka wrote: > With this patch in place, we still need to add `-Xlinker -subsystem:console' > in order

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

awarzynski added a comment. In D125788#3547494 , @sscalpone wrote: > I'm fine with removing or renaming the existing flang shell script. In D125788#3547656 , @rouson wrote: > On Mon, May 30, 2022 at 2:39 AM An

[PATCH] D126291: [flang][Driver] Update link job on windows

awarzynski added a comment. In D126291#391 , @mmuetzel wrote: > ISTR, I somewhere read that Windows isn't a supported host currently. Is this > no longer the case?) Windows is supported: https://lab.llvm.org/buildbot/#/builders/172 :) > If you coul

[PATCH] D126291: [flang][Driver] Update link job on windows

awarzynski added a comment. Thanks for checking @mmuetzel ! In D126291#3555780 , @mmuetzel wrote: > I ended up using these switches: > > cmake \ > -Sllvm \ > -Bbuild \ > -GNinja \ > -DCMAKE_INSTALL_PREFIX=pkg \ > -DCMAKE_C_COMPILER=

[PATCH] D126291: [flang][Driver] Update link job on windows

awarzynski added a comment. Nice! In D126291#3555835 , @mmuetzel wrote: > Is this still a configuration error? No :) Clearly the following `if` block from `tools::addFortranRuntimeLibs` is not entered: if (TC.getTriple().isKnownWindowsMSVCEnvironmen

[PATCH] D126291: [flang][Driver] Update link job on windows

awarzynski added a comment. In D126291#3555966 , @mmuetzel wrote: > In case you don't receive notifications on edits. (Please forgive the noise > if you do.) That's not a problem at all! > After applying this additional patch (not the one in the previo

[PATCH] D126291: [flang][Driver] Update link job on windows

awarzynski added a comment. In D126291#3556324 , @mmuetzel wrote: > With this additional change, I no longer need the `-lc++` flag: Great find, thanks for digging! > Someone should check if that change is sane. But it seems to work for me > (with the "

[PATCH] D126291: [flang][Driver] Update link job on windows

awarzynski added a comment. In D126291#3562542 , @mstorsjo wrote: > In D126291#3562471 , @mmuetzel > wrote: > >> Found why I thought building on Windows wasn't supported: >> https://github.com/llvm/llvm-project

[PATCH] D127207: [flang][driver] Fix support for `-x`

awarzynski created this revision. awarzynski added reviewers: rovka, kiranchandramohan, schweitz, peixin, ekieri, Leporacanthicus. Herald added a reviewer: sscalpone. Herald added projects: Flang, All. awarzynski requested review of this revision. Herald added subscribers: cfe-commits, jdoerfert,

[PATCH] D127207: [flang][driver] Fix support for `-x`

awarzynski added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:268 + // pre-processed inputs. +.Case("f95", Language::Fortran) +.Case("f95-cpp-input", Language::Fortran) rovka wrote: > ekier

[PATCH] D127207: [flang][driver] Fix support for `-x`

awarzynski updated this revision to Diff 435115. awarzynski added a comment. Remove redundant `CHECK` line Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127207/new/ https://reviews.llvm.org/D127207 Files: clang/include/clang/Driver/Options.td

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

awarzynski added a comment. In the call yesterday it was proposed that we add a CMake option that will control the name of the driver. I suggest adding `FLANG_USE_LEGACY_NAME`: - when set to `ON`, the driver binary will be called `flang-new`, - when set to `OFF`, the driver binary will be called

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

awarzynski updated this revision to Diff 435564. awarzynski added a comment. Add the `FLANG_USE_LEGACY_NAME` cmake option that will control the name of the driver executable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125788/new/ https://review

[PATCH] D126291: [flang][Driver] Update link job on windows

awarzynski added a comment. In D126291#3572777 , @rovka wrote: > Moved the check for `-flang-experimental-exec` into > addFortranRuntimeLibraryPath, so it affects all the toolchains. @awarzynski > does this look like a good idea? OK with me! 👍🏻 CHANG

[PATCH] D127207: [flang][driver] Fix support for `-x`

awarzynski added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:268 + // pre-processed inputs. +.Case("f95", Language::Fortran) +.Case("f95-cpp-input", Language::Fortran) ekieri wrote: > awar

[PATCH] D127207: [flang][driver] Fix support for `-x`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3e782ba21be4: [flang][driver] Fix support for `-x` (authored by awarzynski). Changed prior to commit: https://reviews.llvm.org/D127207?vs=435115&i

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

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

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

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

[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 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. 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 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()) +needLocTrackin

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

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

[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/ https://reviews.llvm.o

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

[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". Th

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

[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 clang/li

<    1   2   3   4   5   6   7   >