[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 a comment. In D145845#4186608 , @clementval wrote: > In D145845#4186607 , @awarzynski > wrote: > >> Thanks! >> >> In D145845#4186590 , @clementval >>

[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

[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

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

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

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

[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

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

[PATCH] D143301: [flang] Handle unsupported warning flags

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

[PATCH] D143301: [flang] Handle unsupported warning flags

awarzynski added inline comments. Comment at: clang/include/clang/Driver/Options.td:5045 +// Unsupported flang W options +defm : FlangIgnoredDiagOpt<"extra">; The name of the sub-project is "Flang", the sub-dir is "flang" and the driver name is "flang-new".

[PATCH] D143301: [flang] Handle unsupported warning flags

awarzynski added a comment. @elmcdonough , let me rephrase this (should've been clearer before, sorry): > One thing that's not clear to me - how come "-Wextra" is not treated as an > error and "-Wblah" is? Where's the logic that makes sure that `-Wextra` (and other flags that you redefine

[PATCH] D143301: [flang] Handle unsupported warning flags

awarzynski added a comment. Thanks for the update! One thing that's not clear to me - how come "-Wextra" **is not** treated as an error and "-Wblah" **is**? That's not clear from the code. I'm also realising that I incorrectly assumed that `-Wextra` was defined in Options.td. Instead, it's

[PATCH] D143301: Emit warning for unsupported gfortran flags

awarzynski added a comment. In D143301#4126884 , @elmcdonough wrote: > Thanks for the feedback everyone. This is my current understanding on what I > should do: I am to rename gfortran_unsupported_Group to flang_ignored_w_Group > and move the non W

[PATCH] D143301: Emit warning for unsupported gfortran flags

awarzynski added a comment. In D143301#4126855 , @jdoerfert wrote: > In D143301#4126712 , @awarzynski > wrote: > >>> I think the -W stuff can go in, it has tests and is reasonable. >> >> I'd like for us to rely

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

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

[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

[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

[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

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

[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 , llvm::OptimizationLevel optLevel = defaultOptLevel) { +mlir::PassManager , bool stackArrays = false,

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

[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

[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

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

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

[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

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

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

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

[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

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

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

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

[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

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

[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

[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

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

[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

[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

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

[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] 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] D131475: [Flang] Use find_program() to find clang-tblgen

awarzynski added a comment. Thanks for the quick fix, makes sense!  Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131475/new/ https://reviews.llvm.org/D131475 ___ cfe-commits mailing list

[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

[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

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

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

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

awarzynski added a comment. Many thanks for implementing this! I've only skimmed through so far, but mostly makes sense. Will take a closer look later. Could you update the summary by adding more detail? What options are enabled and whether the semantics from Clang are preserved or not (they

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

awarzynski added a comment. Thanks for working on this. This is not my area of expertise, so I focused on the implementation in the driver. Comment at: clang/include/clang/Driver/Options.td:4897 def ffixed_line_length_VALUE : Joined<["-"], "ffixed-line-length-">, Group,

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

awarzynski updated this revision to Diff 447250. awarzynski added a comment. Change from CamelCase to camelCase in Flang.h so that the function names adhere to LLVM's coding style. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130078/new/

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

awarzynski added a comment. In D130078#3673288 , @MaskRay wrote: > In D130078#3669072 , @awarzynski > wrote: > >> In D130078#3667188 , @MaskRay >> wrote: >> >>>

[PATCH] D129864: [Flang] Generate documentation for compiler flags

awarzynski added a comment. LGTM, thanks! (feel free to address my [nit] when merging or ignore altogether) Comment at: flang/docs/CMakeLists.txt:128 + set(CLANG_TABLEGEN_EXE clang-tblgen) + gen_rst_file_from_td(FlangCommandLineReference.rst -gen-opt-docs

[PATCH] D130254: [CMake][Clang] Copy folder without permissions

awarzynski added a comment. What about similar code in LLVM ? Comment at: clang/cmake/modules/CMakeLists.txt:35 # via CMAKE_MODULE_PATH, place API

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

awarzynski added a comment. In D130078#3667188 , @MaskRay wrote: > `forwardOptions` will be better if you are renaming it anyway. I'd rather create a separate patch and update all other methods to follow LLVM's style. Any idea why the style is not

[PATCH] D129864: [Flang] Generate documentation for compiler flags

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. The change in ClangOptionDocEmitter.cpp is required for Flang as it heavily relies on these "include" flags defined in Options.td

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

awarzynski created this revision. awarzynski added a reviewer: tarunprabhu. 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. The updated name better

[PATCH] D129864: [Flang] Generate documentation for compiler flags

awarzynski added a comment. Makes sense, thanks for working on this! Some minor comments below and inline. From your summary: > This is done by using clang tablegen What do you mean by "clang tablegen"? Is it Clang's clang_tablegen

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

awarzynski updated this revision to Diff 441643. awarzynski added a comment. Add a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125788/new/ https://reviews.llvm.org/D125788 Files: clang/lib/Driver/Driver.cpp

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

awarzynski added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:185 {"flang", "--driver-mode=flang"}, + {"flang-new", "--driver-mode=flang"}, {"clang-dxc", "--driver-mode=dxc"}, richard.barton.arm wrote: > clementval wrote: > >

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

awarzynski added a comment. Thank your for reviewing @clementval ! In D125788#3621585 , @clementval wrote: > Shouldn't we just wait until we can make the permanent renaming so we do not > add unnecessary cmake option? This was discussed in one of our

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGb405407a4899: [clang][flang] Disable defaulting to `-fpie` for LLVM Flang (authored by awarzynski). Changed prior to commit: https://reviews.llvm.org/D128333?vs=440874=440884#toc Repository: rG LLVM

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

awarzynski updated this revision to Diff 440874. awarzynski added a comment. Update the test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128333/new/ https://reviews.llvm.org/D128333 Files: clang/lib/Driver/ToolChains/Linux.cpp

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

awarzynski updated this revision to Diff 440628. awarzynski added a comment. Update the test following the comments from @MaskRay Also added a comment in Linux.cpp and renamed no-pie.f90 as pic-flags.f90 (to avoid FileCheck matching e.g. `! CHECK: pie` against the file name). Repository: rG

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

awarzynski added a comment. In D128333#3613696 , @MaskRay wrote: > True. If it is difficult to override the -pie default from flang side, I am > fine with the code change. Thanks! The proper/long-term fix will require extending Flang's frontend driver

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

awarzynski added a comment. @MaskRay, thank for taking a look! In D128333#3605745 , @MaskRay wrote: > gfortran defaults to PIE as well. While we strive to be compatible with `gfortan`, there's a lot relatively "basic" things still missing in LLVM

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

awarzynski updated this revision to Diff 440283. awarzynski added a comment. Incorporate @clementval 's suggestions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125788/new/ https://reviews.llvm.org/D125788 Files: clang/lib/Driver/Driver.cpp

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

awarzynski added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:185 {"flang", "--driver-mode=flang"}, + {"flang-new", "--driver-mode=flang"}, {"clang-dxc", "--driver-mode=dxc"}, clementval wrote: > Why do we need two lines here?

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

awarzynski added a comment. In D125788#3612533 , @peixin wrote: > In summary: > >> If you want to use the updated name, flang, set FLANG_USE_LEGACY_NAME to ON >> when configuring LLVM Flang. > > OFF? Updated, thanks! Repository: rG LLVM Github

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG869385b11c32: [flang][driver] Add support for `-O{0|1|2|3}` (authored by awarzynski). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128043/new/

[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

[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

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

[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

[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

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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

<    1   2   3   4   5   6   7   >