[PATCH] D64739: [SVE][Inline-Asm] Add support to clang for SVE inline assembly

2019-07-18 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:307 -// 32-bit floating point regsisters +// 32-bit floating point registers "s0", "s1", "s2", "s3", "s4", "s5", "s6", "s7", "s8", "s9", "s10", "s11", You should commit th

[PATCH] D62960: Add SVE opaque built-in types

2019-07-19 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. FWIW, I think the tests look great. Would be nice if someone more experienced with clang could also have a look though. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62960/new/ https://reviews.llvm.org/D62960 ___

[PATCH] D64739: [SVE][Inline-Asm] Add support to specify SVE registers in the clobber list

2019-07-22 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: clang/test/CodeGen/aarch64-sve-inline-asm.c:1 +// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -emit-llvm -target-feature +sve -o - %s | FileCheck %s + sdesmalen wrote: > rovka wrote: > > Can you also add a test without

[PATCH] D62960: SVE opaque type for C intrinsics demo

2019-06-27 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. Just a few nits/suggestions. Comment at: include/clang/Basic/AArch64SVEACLETypes.def:10 +// +// This file defines the database about various builtin singleton types. +// You can be more specific :) Comment at: include/

[PATCH] D62960: SVE opaque type for C intrinsics demo

2019-07-02 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. This looks much better, thanks! Shouldn't there be more tests, e.g. for mangling and maybe the ASTImporter? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62960/new/ https://reviews.llvm.org/D62960 ___

[PATCH] D66524: [SVE][Inline-Asm] Add constraints for SVE predicate registers

2019-09-16 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka added a comment. This revision is now accepted and ready to land. I think all the outstanding comments have been addressed. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66524/new/ https://reviews.llvm.org/D66524 _

[PATCH] D67549: [IntrinsicEmitter] Add overloaded types for SVE intrinsics (Subdivide2 & Subdivide4)

2019-09-16 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: include/llvm/IR/DerivedTypes.h:515 + else +VTy = VectorType::getTruncatedElementVectorType(VTy); +} Why not move this logic to getTruncatedElementVectorType and drop getNarrowerFpElementVectorType altoget

[PATCH] D53137: Scalable vector core instruction support + size queries

2019-09-24 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. > I suspect 'ScalableSize' is the wrong term now; 'TypeSize' may be better. > Thoughts? I agree, TypeSize sounds better. Maybe we can replace the public constructor with 2 static methods, TypeSize::Fixed(Size) and TypeSize::Scalable(Size), so we don't always have to spel

[PATCH] D53137: Scalable vector core instruction support + size queries

2019-10-02 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka marked 2 inline comments as done. rovka added a comment. This revision is now accepted and ready to land. This looks good to me, maybe wait a while to see if anyone else has any further comments. Comment at: llvm/include/llvm/IR/DataLayout.h

[PATCH] D78477: [profile] Don't crash when forking in several threads

2020-06-04 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. Hi! I think this commit is causing some instability on the 10.0.1 branch on 32-bit arm: https://bugs.llvm.org/show_bug.cgi?id=46092 Can you have a look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78477/new/ https://revie

[PATCH] D78477: [profile] Don't crash when forking in several threads

2020-06-11 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. Ping! https://bugs.llvm.org/show_bug.cgi?id=46092 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78477/new/ https://reviews.llvm.org/D78477 ___ cfe-commits mailing list cfe-commit

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

2022-06-27 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka added a comment. Cool, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128043/new/ https://reviews.llvm.org/D128043 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

2022-04-06 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. Why not use the BackendAction for this? There seems to be a lot of shared code, up until the point where you create and use the pass manager (and in the future, when the backend switches to the new pass manager, there will be even more shared code). Co

[PATCH] D122008: [flang][driver] Add support for generating executables

2022-03-22 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka added a comment. This revision is now accepted and ready to land. LGTM :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122008/new/ https://reviews.llvm.org/D122008 __

[PATCH] D123211: [flang][driver] Add support for generating LLVM bytecode files

2022-04-07 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka added a comment. This revision is now accepted and ready to land. In D123211#3433059 , @awarzynski wrote: > Thanks for taking a look Diana! > >> Why not use the BackendAction for this? > > This action only requires the

[PATCH] D122008: [flang][driver] Add support for generating executables

2022-04-07 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. Just for the record, I'd like to speak in favor of option 2 - merge the patch now, but put the functionality behind a flag. This sounds like a good compromise to me. I think if we choose an inconspicuous name like -fexperimentail-test-driver-integration-with-cmake (or som

[PATCH] D123297: [flang][driver] Add support for -mmlir

2022-04-07 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. I don't know the command line library that well, so I have this curiosity: what happens if LLVM and MLIR have 2 different options with the same name? Do we get a compile time error? Or is there a risk that someone might -mllvm -XYZ and it would end up in MLIR's XYZ option

[PATCH] D123297: [flang][driver] Add support for -mmlir

2022-04-07 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: flang/test/Driver/mllvm_vs_mmlir.f90:17 +! MLLVM: flang (LLVM option parsing) [options] +! MLLVM: --print-ir-after-all +! MLLVM-NOT: --mlir-{{.*}} rovka wrote: > Is this the most llvm-ish option we have? I'm concerned that

[PATCH] D123297: [flang][driver] Add support for -mmlir

2022-04-12 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: flang/test/Driver/mllvm_vs_mmlir.f90:17 +! MLLVM: flang (LLVM option parsing) [options] +! MLLVM: --print-ir-after-all +! MLLVM-NOT: --mlir-{{.*}} awarzynski wrote: > awarzynski wrote: > > rriddle wrote: > > > awarzynski w

[PATCH] D123297: [flang][driver] Add support for -mmlir

2022-04-13 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka 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/D123297/new/ https://reviews.llvm.org/D123297

[PATCH] D123967: Disable update_cc_test_checks.py tests in stand-alone builds

2022-04-20 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. I just have a couple of nits, I'll leave it to the clang devs to properly review this. Comment at: clang/test/CMakeLists.txt:17 LLVM_WITH_Z3 + CLANG_BUILT_STANDALONE ) Nit: These seem to be sorted alphabetically. ===

[PATCH] D119012: [flang][driver] Add support for the `-emit-llvm` option

2022-02-07 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: flang/include/flang/Frontend/FrontendOptions.h:37 + /// Emit a .llvm file + EmitLLVM, Shouldn't this be .ll? Comment at: flang/lib/Frontend/FrontendActions.cpp:421 + // Set-up the MLIR pass manager

[PATCH] D119012: [flang][driver] Add support for the `-emit-llvm` option

2022-02-08 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka added a comment. This revision is now accepted and ready to land. LGTM, thanks! I just have a minor nit, feel free to ignore it. Comment at: flang/lib/Frontend/FrontendActions.cpp:471 + + if (!ci.IsOutputStreamNull()) { +llvmModule->prin

[PATCH] D119918: [CMake] Rename TARGET_TRIPLE to LLVM_TARGET_TRIPLE

2022-02-16 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. Is this going to break for everyone that still passes TARGET_TRIPLE to cmake? If so, we should probably have a period where we support both. In particular, this might break some of our buildbots

[PATCH] D119918: [CMake] Rename TARGET_TRIPLE to LLVM_TARGET_TRIPLE

2022-02-17 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka added a comment. I'm happy if the buildbots are happy :D thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119918/new/ https://reviews.llvm.org/D119918 ___ cfe-com

[PATCH] D124667: [flang][driver] Add support for consuming LLVM IR/BC files

2022-05-02 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka added a comment. This revision is now accepted and ready to land. A few nits, but otherwise LGTM. Comment at: flang/lib/Frontend/FrontendActions.cpp:86 + + // ... otherwise, generate an MLIR module from the input Fortran source bool res =

[PATCH] D124669: [flang][driver] Add support for -save-temps

2022-05-02 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: clang/include/clang/Driver/Options.td:4131 def : Flag<["-"], "no-integrated-as">, Alias, - Flags<[CC1Option, NoXarchOption]>; + Flags<[CC1Option,FlangOption,NoXarchOption]>; awarzynski wrote: > unterumarmung w

[PATCH] D124669: [flang][driver] Add support for -save-temps

2022-05-03 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. I think I confused myself yesterday, it does make sense to add -fno-integrated-as for this. Could we add a test for it independent of save-temps? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124669/new/ https://reviews.llv

[PATCH] D124669: [flang][driver] Add support for -save-temps

2022-05-04 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: flang/test/Driver/fno-integrated-as.f90:6 +!-- +! Verify that there _is_ a seperate line with an assembler invocation +! RUN: %flang -c -fno-integrated-as %s -### 2>&1 | FileCheck %s

[PATCH] D124669: [flang][driver] Add support for -save-temps

2022-05-05 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka added a comment. This revision is now accepted and ready to land. LGTM, thanks! It would be nice to rebase the patch and see the pre-commit CI passing, but then again you're the one dealing with the buildbots if you break anything, so do as you prefer :) Rep

[PATCH] D125628: [flang][driver] Add support for generating executables on MacOSX/Darwin

2022-05-16 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka added a comment. This revision is now accepted and ready to land. LGTM, I'll try to send one for Windows this week. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125628/new/ https://reviews.llvm.org/D125628

[PATCH] D125957: [flang][driver] Make driver accept `-module-dir`

2022-05-19 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka added a comment. This revision is now accepted and ready to land. Seems legit, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125957/new/ https://reviews.llvm.org/D125957 __

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

2022-05-24 Thread Diana Picus via Phabricator via cfe-commits
rovka created this revision. rovka added reviewers: awarzynski, kiranchandramohan. rovka added a project: Flang. Herald added a subscriber: jdoerfert. Herald added a reviewer: sscalpone. Herald added a project: All. rovka requested review of this revision. Herald added subscribers: cfe-commits, Mas

[PATCH] D66524: [SVE][Inline-Asm] Add constraints for SVE predicate registers

2019-09-02 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. Just some drive-by suggestions :) Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:5747 + +PredicateConstraint isPredicateConstraint(StringRef Constraint) { + PredicateConstraint P = PredicateConstraint::Invalid; Nit: I think get-

[PATCH] D136090: Handle errors in expansion of response files

2022-10-21 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: clang/test/Driver/config-file-errs.c:16 // RUN: not %clang --config somewhere/nonexistent-config-file 2>&1 | FileCheck %s -check-prefix CHECK-NONEXISTENT -// CHECK-NONEXISTENT: configuration file '{{.*}}somewhere/nonexistent-config-file

[PATCH] D136090: Handle errors in expansion of response files

2022-10-21 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. Nice tests! :) I have one microscopic nit and one question (because I don't know an awful lot about config files). Thanks for working on this. Comment at: llvm/lib/Support/CommandLine.cpp:1188 + // macros. + if (!RelativeNames && !InConfigFile) re

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

2022-09-20 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. IMO this is a reasonable approach, I only have a few nits. Comment at: flang/runtime/environment.cpp:42 +#else +if (setenv(name, value, 0) == -1) { +#endif Comment at: flang/runtime/environment.cpp:77 +#else + env

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

2022-09-29 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Herald added a subscriber: zero9178. Comment at: flang/test/Driver/convert.f90:1 +! Ensure argument -fconvert= works as expected. + jpenix-quic wrote: > awarzynski wrote: > > rovka wrote: > > > Nit: Shouldn't you also check that valid

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

2022-06-13 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. 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. When specifying a triple, we need to provide an architecture. Leaving the triple as `unkown-linux-

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

2022-06-13 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 436300. rovka added a comment. Herald added a subscriber: ormris. - Switch to the new style of testing (using triples rather than separate files) - Fix oversight in Darwin toolchain file I hope I've addressed all the comments and nothing slipped past during reb

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

2022-06-13 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 436301. rovka added a comment. Missed a spot (removing the namespace in MinGW.cpp) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126291/new/ https://reviews.llvm.org/D126291 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp clang/lib/Driver/ToolC

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

2022-06-13 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. In D126291#3572796 , @mmuetzel wrote: > In D126291#3572777 , @rovka wrote: > >> Moved the check for `-flang-experimental-exec` into >> addFortranRuntimeLibraryPath, so it affects all the to

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

2022-06-13 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. In D126291#3577159 , @mstorsjo wrote: > In D126291#3577133 , @rovka wrote: > >> I had the same idea about switching the tests to using target triples >> instead of having separate files for

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

2022-06-13 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. I'm guessing the Windows precommit is failing because --ld-path is ignored on Windows, even if we use a Linux target? I have a fix that works on my Windows machine, coming right up. Comment at: flang/test/Driver/linker-flags.f90:28 +! GNU-SAME: -lFortra

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

2022-06-13 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 436340. rovka added a comment. - Stop using --ld-path, since it seems to be ignored on Windows. Instead, just match any path that ends in ld (should work with ld, lld, gold). This isn't very robust, but I don't have any better ideas (other than actually suppor

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

2022-06-14 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 436687. rovka added a comment. Clarified test checks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126291/new/ https://reviews.llvm.org/D126291 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp clang/lib/Driver/ToolChains/CommonArgs.h clang/li

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

2022-06-14 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: flang/test/Driver/linker-flags.f90:51 +! MSVC-NOT: libcmt +! MSVC-NOT: oldnames +! MSVC-SAME: "[[object_file]]" mmuetzel wrote: > Lines 50-51 seem to be duplicates of lines 44-45. Is this intentional? Yes, I don't want tho

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

2022-06-14 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: flang/test/Driver/linker-flags.f90:51 +! MSVC-NOT: libcmt +! MSVC-NOT: oldnames +! MSVC-SAME: "[[object_file]]" awarzynski wrote: > rovka wrote: > > mmuetzel wrote: > > > Lines 50-51 seem to be duplicates of lines 44-45. I

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

2022-06-14 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 436722. rovka added a comment. - Use --implicit-not-check <3 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126291/new/ https://reviews.llvm.org/D126291 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp clang/lib/Driver/ToolChains/CommonArgs.h c

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

2022-06-15 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: flang/test/Driver/linker-flags.f90:51 +! MSVC-NOT: libcmt +! MSVC-NOT: oldnames +! MSVC-SAME: "[[object_file]]" awarzynski wrote: > rovka wrote: > > awarzynski wrote: > > > rovka wrote: > > > > mmuetzel wrote: > > > > > Li

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

2022-06-17 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. I'm going to go ahead and commit this on Monday if nobody raises any objections until then. Thanks again to everyone for all the help & feedback! Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:773 + // TODO: Make this work unconditionally once F

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

2022-06-20 Thread Diana Picus via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG26041e17006c: Update link job for flang on windows (authored by rovka). Changed prior to commit: https://reviews.llvm.org/D126291?vs=436722&id=438

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

2022-06-22 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: flang/include/flang/Frontend/CodeGenOptions.def:12 +// Optionally, the user may also define ENUM_CODEGENOPT (for options +// that have enumeration type and VALUE_CODEGENOPT is a code +// generation option that describes a value rather than

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

2022-06-23 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. I just realized I haven't pestered you enough about testing :) Can you add a test that -O4 indeed warns and uses -O3? Also, the summary says this should work in both the compilation and the frontend driver, but you're only testing with %flang_fc1. Comm

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

2022-05-27 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. In D126291#3536718 , @kiranchandramohan wrote: > @rovka in case you missed this, the test is failing in windows. Yeah, thanks, I was out of office a bit so I couldn't get to it sooner. I'll give it a try without lld. Repository

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

2022-05-31 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 433054. rovka added a comment. Updated test to check for link.exe instead of lld-link.exe. This doesn't look great but it works for both lld-link.exe and the default link.exe. I tried to use --ld-path=various/paths instead, but it seems to be ignored on Window

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

2022-05-31 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: flang/test/Driver/flang-linker-flags-windows.f90:16 +! Linker invocation to generate the executable +! CHECK-LABEL: lld-link.exe +! CHECK-NOT: libcmt Meinersbur wrote: > This is failing for me. Instead of `lld-link.exe`,

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

2022-06-01 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. In D126291#3547142 , @awarzynski wrote: > In D126291#3547023 , @rovka wrote: > >> With this patch in place, we still need to add `-Xlinker -subsystem:console' >> in order to compile a simp

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

2022-06-01 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 433318. rovka added a comment. Updated comment in `linker-flags.f90` test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126291/new/ https://reviews.llvm.org/D126291 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp clang/lib/Driver/ToolChains/Com

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

2022-06-02 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. In D126291#3550563 , @Meinersbur wrote: > While the test passes now, I still get `LINK : fatal error LNK1561: entry > point must be defined` when trying to actually link. Isn't it expected to not > work yet? > > Linking with `lld

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

2022-06-02 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:753 + llvm::opt::ArgStringList &CmdArgs) { + if (TC.getTriple().isOSWindows()) { +CmdArgs.push_back("Fortran_main.lib"); mmuetzel wrote: > Ar

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

2022-06-03 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 433988. rovka added a comment. - Update for MinGW. - Add `/subsystem:console` to help `link.exe` understand what's going on. Thanks for all the comments. I don't have a MinGW environment readily available for testing. @mmuetzel Could you test this? Alternative

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

2022-06-03 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. @mmuetzel Thanks for looking into this! I think since you're passing `-DCLANG_DEFAULT_RTLIB=compiler-rt`, you might indeed need to build compiler-rt (or at least the builtins part of it). FYI, I'll be out of office until Wednesday, but I'll definitely check all the comme

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

2022-06-08 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka added a comment. This revision is now accepted and ready to land. LGTM, thanks. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:268 + // pre-processed inputs. +.Case("f95", Language::Fortran) +

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

2022-06-08 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 435150. rovka edited the summary of this revision. rovka added a comment. - Fixed test - Unconditionally added the subsystem arg - Incorporated the MinGW toolchain changes (Thanks again @mmuetzel, I'm adding you as co-author) @awarzynski I agree that the other

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

2022-06-10 Thread Diana Picus via Phabricator via cfe-commits
rovka added inline comments. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:133 + if (C.getDriver().IsFlangMode()) { +tools::addFortranRuntimeLibraryPath(TC, Args, CmdArgs); mmuetzel wrote: > The GNU toolchain has this conditional on > `Args.hasArg(opti

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

2022-06-10 Thread Diana Picus via Phabricator via cfe-commits
rovka updated this revision to Diff 435842. rovka added a comment. Moved the check for `-flang-experimental-exec` into addFortranRuntimeLibraryPath, so it affects all the toolchains. @awarzynski does this look like a good idea? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126291/new/

[PATCH] D35884: Update to use enum classes for various ARM *Kind enums

2017-07-27 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka added a comment. This revision is now accepted and ready to land. Looks entirely mechanical, I don't see any problem with it (just a couple of nits). Comment at: lib/Basic/Targets/AArch64.cpp:94 llvm::AArch64::parseCPUArch(Name) !=

[PATCH] D31709: [NFC] Refactor DiagnosticRenderer to use FullSourceLoc

2017-04-20 Thread Diana Picus via Phabricator via cfe-commits
rovka accepted this revision. rovka added a comment. This revision is now accepted and ready to land. I don't see anything wrong with this, I think you can commit it in a couple of days if nobody comes up with a reason why the DiagnosticRenderer shouldn't use FullSourceLoc. https://reviews.llv

[PATCH] D120246: [flang][driver] Add support for `--target`/`--triple`

2022-02-22 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. Nit: Should we also have a test for print-effective-triple? Otherwise LGTM, although I'm not sure that -emit-llvm is necessarily something we'd want flang users to be exposed to. Comment at: flang/include/flang/Frontend/TargetOptions.h:20 +/// Options f

[PATCH] D120568: [flang][driver] Add support for -S and implement -c/-emit-obj

2022-02-28 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. I'm having trouble building this on Windows on Arm. I'm still investigating if it's a problem with the patch or just growing pains on the WoA side :) I'll try to get to the bottom of it as soon as I can. Comment at: flang/lib/Frontend/FrontendActions.cp

[PATCH] D120568: [flang][driver] Add support for -S and implement -c/-emit-obj

2022-03-01 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. This passes on Windows on Arm if you add a forward declaration for `class PassInstrumentation` here . This is necessary because otherwise cl

[PATCH] D120568: [flang][driver] Add support for -S and implement -c/-emit-obj

2022-03-01 Thread Diana Picus via Phabricator via cfe-commits
rovka added a comment. I don't have any other comments, I'll let Eric take it from here :) Comment at: flang/lib/Frontend/FrontendActions.cpp:509 + TargetRegistry::lookupTarget(theTriple, error); + assert(theTarget && "Failed to create Target"); + awarzyn