[clang] [flang] [lldb] [llvm] [mlir] Fix typo "instrinsic" (PR #112899)
https://github.com/banach-space approved this pull request. LGTM, thanks! I blame my editor's dictionary 😂 https://github.com/llvm/llvm-project/pull/112899 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [flang][driver] rename flang-new to flang (PR #110023)
@@ -339,11 +335,11 @@ just added using your new frontend option. ## CMake Support As of [#7246](https://gitlab.kitware.com/cmake/cmake/-/merge_requests/7246) -(and soon to be released CMake 3.24.0), `cmake` can detect `flang-new` as a +(CMake 3.28.0), `cmake` can detect `flang` as a banach-space wrote: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/7246 pre-dates CMake 3.28.0 by quite a few releases, I would just remove the link. @DavidTruby what makes 3.28 special? Should 3.28 be marked as requirement? https://github.com/llvm/llvm-project/pull/110023 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Make -stdlib= option visible to flang and silently ignored by it (PR #110598)
banach-space wrote: We should avoid dummy flags like this - they are usually an indication of a larger issue. I don't want to block this and, as @Meinersbur points out, there are some "inherited issues" to consider too. However, I'd like to identify the right long-term solution here. If possible. > I've specified the -stdlib=libc++ option globally, in the CXXFLAGS and > LDFLAGS variables Why are CXXFLAGS used when invoking a Fortran compiler? That's not correct, is it? `LDFLAGS` would make sense, but then `-stdlib=` should be treated as a linker flag and `flang-new` as a linker driver. In fact, it sounds like `-stdlib=` could fall into some special category: * valid when **compiling** C++, * invalid when **compiling** Fortran, * valid when **linking** mixed C++ and Fortran object files. To me it sounds like: * flags from `CXXFLAGS` should not be used in Fortran _compilation_ flows, * `-stdlib=` should be flagged as a linker option (that seems to be missing?), * `flang-new` should allow/ignore "linker" flags that are not "visible" in Fortran _compilation_ flows (as in, the "Visibility" logic could be tweaked). If `-stdlib=` were to be made available in Fortran compilation, the help-text should be updated accordingly. https://github.com/llvm/llvm-project/pull/110598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Rename `flang-new` as `flang` (PR #74377)
https://github.com/banach-space closed https://github.com/llvm/llvm-project/pull/74377 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Rename `flang-new` as `flang` (PR #74377)
banach-space wrote: Closing in favour of #110023 https://github.com/llvm/llvm-project/pull/74377 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [flang][driver] rename flang-new to flang (PR #110023)
@@ -339,11 +335,11 @@ just added using your new frontend option. ## CMake Support As of [#7246](https://gitlab.kitware.com/cmake/cmake/-/merge_requests/7246) -(and soon to be released CMake 3.24.0), `cmake` can detect `flang-new` as a +(and soon to be released CMake 3.24.0), `cmake` can detect `flang` as a banach-space wrote: > It seems like the check is through preprocessor macros defined by the > different compilers. Yes, thank you @hakostra 🙏🏻 https://github.com/llvm/llvm-project/pull/110023 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Enable the -B option (PR #109965)
https://github.com/banach-space approved this pull request. LGTM, thanks! Looks like you've addressed all comments, so feel free to land it. https://github.com/llvm/llvm-project/pull/109965 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [flang][driver] rename flang-new to flang (PR #110023)
banach-space wrote: Having multiple active PR's for one change is IMHO rather confusing. It creates unnecessary duplication - why do we expect people to review the same change twice? As a case in point: > Since we are making this change now, should this PR be updated to follow > clang's scheme of having clang point to clang-$version? That was already included in #74377. It is absolutely secondary who commits this change (just acknowledging that there are two implementations). However, please make sure that: * there is a well documented transition period during which `flang-new` is still available as a sym-link (please define _when_ such transition period would end), * we use this as an opportunity to improve Flang's consistency with Clang (please introduce `flang-$version`), * this change does not break Flang's buildbots and all customers are well supported throughout the renaming (there might be a stream of bug reports following this), * community members who contributed to the previous PRs for this (either through code, reviews or testing) are aware that the discussion has moved here and all changes have been attributed accordingly. Thank you all for working on this! 🙏🏻 https://github.com/llvm/llvm-project/pull/110023 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Add pre-processing type for Fortran pre-processed files (PR #104664)
@@ -79,7 +79,17 @@ TYPE("c++-module-cpp-output",PP_CXXModule, INVALID, "iim",phases TYPE("ada", Ada, INVALID, nullptr, phases::Compile, phases::Backend, phases::Assemble, phases::Link) TYPE("assembler",PP_Asm, INVALID, "s", phases::Assemble, phases::Link) TYPE("assembler-with-cpp", Asm, PP_Asm, "S", phases::Preprocess, phases::Assemble, phases::Link) -TYPE("f95", PP_Fortran, INVALID, "i", phases::Compile, phases::Backend, phases::Assemble, phases::Link) + +// Note: The `phases::Preprocess` phase is added to ".i" (i.e. Fortran +// pre-processed) files. The reason is that the pre-processor "phase" has to be +// re-run to make sure that e.g. the include flags (i.e. `-I `) are +// preserved. That's because these include paths will contain module files and, +// unlike C header files, these module files wouldn't be included in the +// pre-processed file. In particular, we need to add the search paths for these +// modules when flang needs to emits pre-processed files. Therefore, the +// `PP_TYPE` is set to `PP_Fortran` so that the driver is fine with +// "pre-processing a pre-processed file". banach-space wrote: [nits] ```suggestion // Note: The `phases::Preprocess` phase is added to ".i" (i.e. Fortran // pre-processed) files. The reason is that the pre-processor "phase" has to be // re-run to make sure that e.g. the include flags (i.e. `-I `) are // preserved. That's because these include paths will contain module files and, // unlike C header files, these module files wouldn't be included in the // pre-processed file. In particular, we need to add the search paths for these // modules when Flang needs to emit pre-processed files. Therefore, the // `PP_TYPE` is set to `PP_Fortran` so that the driver is fine with // "pre-processing a pre-processed file". ``` https://github.com/llvm/llvm-project/pull/104664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Add pre-processing type for Fortran pre-processed files (PR #104664)
https://github.com/banach-space edited https://github.com/llvm/llvm-project/pull/104664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Add pre-processing type for Fortran pre-processed files (PR #104664)
https://github.com/banach-space approved this pull request. LGTM, thanks! https://github.com/llvm/llvm-project/pull/104664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] Allow disabling of types from the command line (PR #107126)
https://github.com/banach-space commented: What's the end goal here? To add an option for every possible bit-width? If yes, then these options should take bit width as parameters. Btw, does GFortran implement anything like this? If yes, are you making sure that the semantics in Flang are consistent? Please also add help text. https://github.com/llvm/llvm-project/pull/107126 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] support -fno-openmp (PR #107087)
https://github.com/banach-space approved this pull request. https://github.com/llvm/llvm-project/pull/107087 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Add pre-processing type for Fortran pre-processed files (PR #104664)
@@ -79,7 +79,14 @@ TYPE("c++-module-cpp-output",PP_CXXModule, INVALID, "iim",phases TYPE("ada", Ada, INVALID, nullptr, phases::Compile, phases::Backend, phases::Assemble, phases::Link) TYPE("assembler",PP_Asm, INVALID, "s", phases::Assemble, phases::Link) TYPE("assembler-with-cpp", Asm, PP_Asm, "S", phases::Preprocess, phases::Assemble, phases::Link) -TYPE("f95", PP_Fortran, INVALID, "i", phases::Compile, phases::Backend, phases::Assemble, phases::Link) + +// Note: The `phases::Preprocess` phase is added to ".i" (i.e. +// Fortran pre-processed) files. The reason is that Fortran +// pre-processed files need further pre-proecessing when they +// include modules from non-standard paths. In particular, we +// need to add the search paths for these modules when flang +// needs to emits pre-processed files. banach-space wrote: > (...) The reason is that Fortran // pre-processed files Could you clarify that this isn't really about "Fortran pre-processed files", but more about "Fortran files pre-processed by LLVM Flang"? > need further pre-proecessing when they // include modules from non-standard paths This is a bit misleading. IIUC, no further pre-processing happens, but the pre-processor "phase" has to be re-run to make sure that e.g. the include flags (i.e. `-I `) are preserved. That's because these include paths will contain module files (could `-I ` define any other dirs?) and, unlike C header files, these module files wouldn't be included in the pre-processed file. Please also add a note that `PP_TYPE` is set to `PP_Fortran` so that the driver is fine with "pre-processing a pre-processed file". Hope this makes sense and apologies if I come across as pedantic. I just find this quite tricky and want to make sure that we throughly document these quirks for our future selves :) https://github.com/llvm/llvm-project/pull/104664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Add pre-processing type for Fortran pre-processed files (PR #104664)
https://github.com/banach-space edited https://github.com/llvm/llvm-project/pull/104664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Add pre-processing type for Fortran pre-processed files (PR #104664)
https://github.com/banach-space commented: Thanks for the updates, some minor suggestions inline. https://github.com/llvm/llvm-project/pull/104664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Add support for -mllvm -print-pipeline-passes (PR #106141)
https://github.com/banach-space edited https://github.com/llvm/llvm-project/pull/106141 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Add support for -mllvm -print-pipeline-passes (PR #106141)
https://github.com/banach-space approved this pull request. Approving with the caveat that we should avoid leveraging LLVM options like this and that long-term this should be refactored as a dedicated, first-class driver option. This can be implemented in a seperate PR. Thank you for the discussion for addressing my comment @tarunprabhu 🙏🏻 LGTM https://github.com/llvm/llvm-project/pull/106141 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Add pre-processing type to `.i` (pre-processed) files (PR #104664)
banach-space wrote: Apologies for the delay, I am OOO 😅 I've had another look and I think that I better understand what the issue is. It is important to distinguish two input types that we are dealing with here: * `-x f95-cpp-input` - Fortran input _before_ pre-processing, * `-x f95` - Fortran input _after_ pre-processing This PR updates the definition of the latter to also include pre-processing, which is counter-intuitive. However, given that the pre-processor is something unique to every driver (i.e. not defined by the standard itself), some flexibility is obviously required. In this case, IIUC, the driver makes an assumption that **is valid for C-like inputs**, but **invalid for Fortran inputs**. Specifically, it assumes that _after_ preprocessing, no pre-processing is required and drops all the flags considered as specific to pre-processing (e.g. `-I` flags). While that works for C header files (which get "copied" into files that include them), that's not the case for Fortran modules. At least not today. Also, Flang's pre-processor is always run anyway (we only really "emulate" the split into pre and post-processing phases). IIUC, this is consistent with your understanding - I want to make sure that we are on the same page and that I am not missing anything myself. If you agree with the above, then this PR is more about updating the definition of "pre-processed" Fortran files rather than e.g. fixing `-save-temps` or "*.i" files. It would be good to: * Rephrase the summary to highlight that (e.g. "add pre-processing phase phase for Fortran pre-processed files"). * Add a comment in Types.def to explain the rationale behind ... having to pre-process files that have already been pre-processed. How does it sound? https://github.com/llvm/llvm-project/pull/104664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Add pre-processing type to `.i` (pre-processed) files (PR #104664)
banach-space wrote: For context, this PR documents the origins of .i in the context of `flang-new`: * https://github.com/llvm/llvm-project/commit/a65afce731c2fa91effcfa5d4917d6ad90a28bf0 If these files are already pre-processed, why would we add `phases::Preprocess` to the list of compilation phases? Also, it's still unclear to me what issue is: > This solves a bug when using --save-temps with source files that include > modules from non-standard directories IIRC, the generated temp files (e.g. *.i) are used for the subsequent compilation phases. So, after pre-processing file.f90 we'd get file.i with all the modules included, no? So `-I` shouldn't be a concern, right? Please bear with me, it's been a while since I've touched this and also, I am afk 😅 https://github.com/llvm/llvm-project/pull/104664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Add pre-processing type to `.i` files (PR #104664)
banach-space wrote: Hi Kareem, thanks for the fix! Is there any reference that would document .i files? Perhaps GFortran docs? This might be sth standard, but we should capture that in our docs nonetheless (eg in a commit summary). Btw, I will be OOO for a while, so apologies in advance if there's a delay from my side. https://github.com/llvm/llvm-project/pull/104664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][cuda][driver] Make sure flang does not switch to cc1 (PR #104613)
https://github.com/banach-space approved this pull request. LGTM, thanks! https://github.com/llvm/llvm-project/pull/104613 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][cuda][driver] Make sure flang does not switch to cc1 (PR #104613)
@@ -12,4 +13,8 @@ program main ! CHECK: INTEGER :: var = 1 ! CHECK: INTEGER, DEVICE :: dvar -! ERROR: cuda-option.f90:8:19: error: expected end of statement +! ERROR: cuda-option.f90:{{.*}}:{{.*}}: error: expected end of statement + +! The whole pipeline is not in place yet. It will currently fails at MLIR banach-space wrote: ```suggestion ! The whole pipeline is not in place yet. It will currently fail at MLIR ``` nit :) https://github.com/llvm/llvm-project/pull/104613 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][cuda][driver] Make sure flang does not switch to cc1 (PR #104613)
https://github.com/banach-space edited https://github.com/llvm/llvm-project/pull/104613 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Restrict Ofast deprecation help message to Clang (PR #101682)
https://github.com/banach-space approved this pull request. https://github.com/llvm/llvm-project/pull/101682 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
https://github.com/banach-space approved this pull request. Thanks for addressing my feedback, LGTM https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
@@ -492,6 +493,18 @@ void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs, if (Args.hasArg(options::OPT_nogpulib)) CmdArgs.push_back("-nogpulib"); } + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + if (JA.isHostOffloading(Action::OFK_OpenMP)) { +SmallString<128> Targets("-fopenmp-targets="); banach-space wrote: This looks to be taken verbatim from Clang: * https://github.com/llvm/llvm-project/blob/2ba3fe7356f065757a2279f65e4ef5c8f1476293/clang/lib/Driver/ToolChains/Clang.cpp#L7768-L7778 It should be extracted to a dedicated hook and moved to https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/CommonArgs.cpp. That's the best we have today. > I think @banach-space is the expert on this area, so maybe he knows of any > plans already in place to improve this situation or can give us some pointers > on how to better deal with this in the short term. These days I only have very limited bandwidth to _review_ driver patches. Please consider every contribution that you make as an opportunity to improve code re-use between Clang and Flang. https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang-new][OpenMP] Add bitcode files for AMD GPU OpenMP (PR #96742)
https://github.com/banach-space approved this pull request. https://github.com/llvm/llvm-project/pull/96742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang-new][OpenMP] Add bitcode files for AMD GPU OpenMP (PR #96742)
banach-space wrote: > > > > Who could be the right person to ask? > > > > > > > > > I don't know. Open-source LLVM Flang meetings can be good place to ask > > > this question. > > > > > > Did you ask? What feedback did you get? > > @banach-space I asked question on flang-slack, I mentioned the issue on the > latest Flang technical meeting and I described potential solution here: > https://discourse.llvm.org/t/offloading-on-nvptx64-target-with-flang-new-leads-to-undefined-reference-s/80237 > . I got no feedback. > > Can I merge this PR? The issue with -`fcuda-is-device` is resolved. If you > wish I can extend driver checks for `-mlink-builtin-bitcode` as a separate PR. Thanks for following-up! Overall this looks good to me, but please update the summary with some details/context, e.g. * why are you adding `-nogpulib` in tests? * "Flang-new needs to add mlink-builtin-bitcode" - please add a note explaining that this flag is added by `addClangTargetOptions()` (that wasn't obvious to me). Approving as is, no need to wait for me to take another look. https://github.com/llvm/llvm-project/pull/96742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
banach-space wrote: Given the growing number of OpenMP and/or "offloading" flags, I agree with @AnastasiaStulova that it would be good to clarify the overall goal/design. That's not clear to me. Is there are reference implementation that Flang is meant to follow? For example Clang or GFortran? Could we re-use more of Clang logic here? There's also a question how various flags impact one another and that seems also confusing (see e.g. this [question](https://flang-compiler.slack.com/archives/C01PY03PP9P/p1721378390015629) by @DominikAdamski ). Now, this is not really my area of expertise and I tend to stay hands-off. I rely on @jhuber6's [expertise in the area](https://llvm.org/devmtg/2022-04-03/slides/Improving.the.OpenMP.Offloading.Driver.LTO.libraries.and.toolchains.pdf) :) But it does sound like it would be good to discuss the long term goals here. That's not a blocker for me for this particular change. https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Enable config file options (PR #100343)
https://github.com/banach-space approved this pull request. https://github.com/llvm/llvm-project/pull/100343 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Introduce -fopenmp-targets offloading option (PR #100152)
@@ -492,6 +493,18 @@ void Flang::addOffloadOptions(Compilation &C, const InputInfoList &Inputs, if (Args.hasArg(options::OPT_nogpulib)) CmdArgs.push_back("-nogpulib"); } + + // For all the host OpenMP offloading compile jobs we need to pass the targets + // information using -fopenmp-targets= option. + if (JA.isHostOffloading(Action::OFK_OpenMP)) { +SmallString<128> Targets("-fopenmp-targets="); banach-space wrote: Or, even better, `StringLiteral`. https://github.com/llvm/llvm-project/pull/100152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Add -rtlib flag (PR #99058)
https://github.com/banach-space approved this pull request. https://github.com/llvm/llvm-project/pull/99058 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] Add basic -mtune support (PR #98517)
https://github.com/banach-space closed https://github.com/llvm/llvm-project/pull/98517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] Add basic -mtune support (PR #98517)
https://github.com/banach-space approved this pull request. https://github.com/llvm/llvm-project/pull/98517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Add -fhermetic-module-files (PR #98083)
https://github.com/banach-space approved this pull request. The driver logic makes sense, thank you! LGTM https://github.com/llvm/llvm-project/pull/98083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang-new][OpenMP] Add bitcode files for AMD GPU OpenMP (PR #96742)
@@ -333,6 +333,9 @@ void Flang::AddAMDGPUTargetArgs(const ArgList &Args, StringRef Val = A->getValue(); CmdArgs.push_back(Args.MakeArgString("-mcode-object-version=" + Val)); } + + const ToolChain &TC = getToolChain(); + TC.addClangTargetOptions(Args, CmdArgs, Action::OffloadKind::OFK_OpenMP); banach-space wrote: > Shall I extend https://github.com/llvm/llvm-project/pull/94763 ? Yes, please. > Who could be the right person to ask? > > I don't know. Open-source LLVM Flang meetings can be good place to ask this > > question. Did you ask? What feedback did you get? https://github.com/llvm/llvm-project/pull/96742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang-new][OpenMP] Add bitcode files for AMD GPU OpenMP (PR #96742)
@@ -8024,7 +8024,7 @@ def source_date_epoch : Separate<["-"], "source-date-epoch">, // CUDA Options //===--===// -let Visibility = [CC1Option] in { +let Visibility = [CC1Option, FC1Option] in { banach-space wrote: This enables 4 CUDA flags in `flang-new -fc1` - this really shouldn't be needed in this PR. I would expect that AMD GPU and CUDA logic to be orthogonal. If that's not the case, then lets try to either fix or at document that. I appreciate that some sub-optimal behaviour might be inherited from Clang. In such cases, lets try to: 1. Understand what's going on. 2. If a path towards a fix is clear, lets just fix it. 3. If there's no easy fix, lets document this with a GitHub issue and add a link in the commit/code. Thanks! https://github.com/llvm/llvm-project/pull/96742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang-new][OpenMP] Add bitcode files for AMD GPU OpenMP (PR #96742)
banach-space wrote: > Would it be possible for you to investigate that? It really shouldn't be > required if we can't help it. +1 https://github.com/llvm/llvm-project/pull/96742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)
@@ -0,0 +1,9 @@ +; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s banach-space wrote: I have X86 disabled - I just double checked. And yes, this test works for me just fine without the guard. Alexis, what error do you get when it fails for you? To me, it would make sense if `mlir-translate` didn't care - it doesn't have to interpret these attributes, does it? Btw @AlexisPerry , there's no need to clone "tune-cpu.f90" as [tune-cpu-llvm-aarch.f90](https://github.com/llvm/llvm-project/pull/96688/files#diff-879569867a123346813864ce36bc15249acb8b57bc335c808eb4d30818ec8685). Instead, you can use conditional RUN lines like here: * https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/pic-flags.f90 https://github.com/llvm/llvm-project/pull/96688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)
@@ -0,0 +1,9 @@ +; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s banach-space wrote: Hm, ninja `check-mlir` worked for me just fine on Aarch64 🤔 I think that `mlir-translate` doesn't really care :) Put differently, `REQUIRES` is not needed here. https://github.com/llvm/llvm-project/pull/96688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang-new][OpenMP] Add offload related flags for AMDGPU (PR #96742)
banach-space wrote: > Clang for AMDGPU supports OpenMP and > [HIP](https://clang.llvm.org/docs/HIPSupport.html) and it reuses the same > code. For example `-fcuda-is-device` flag needs to be checked for [legacy HIP > host > code](https://github.com/llvm/llvm-project/blob/2033b1cf16f040e1369d8efba8439dcd3e36ed31/clang/lib/Basic/Targets/AMDGPU.cpp#L278). > Thanks! I'm still puzzled though: > In the future it will be needed for Flang equivalent functions: > AMDGPUTargetCodeGenInfo::getGlobalVarAddressSpace > AMDGPUTargetInfo::getTargetDefines Why would `-fcuda-is-device` be required? From your link I gather that the AMD logic in Clang simply makes sure that `-fcuda-is-device` wasn't used? > I would like to reuse the same part of the AMD GPU toolchain for Flang. That would be great - what's the plan here then? Simply to rely on the code in Clang? Also, note that that's `TargetInfo` (which lives in `clangBasic`) rather than `Toolchain` (that lives in `clangDriver`). This is actually key because it makes the coupling between Flang and Clang even stronger. https://github.com/llvm/llvm-project/pull/96742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang-new][OpenMP] Add offload related flags for AMDGPU (PR #96742)
@@ -333,6 +333,9 @@ void Flang::AddAMDGPUTargetArgs(const ArgList &Args, StringRef Val = A->getValue(); CmdArgs.push_back(Args.MakeArgString("-mcode-object-version=" + Val)); } + + const ToolChain &TC = getToolChain(); + TC.addClangTargetOptions(Args, CmdArgs, Action::OffloadKind::OFK_OpenMP); banach-space wrote: > Clang does not verify how we use these flags and it accepts them for non-GPU > target. It's OK to make Flang "stricter" if we believe that's the right thing to do ;-) (I think that generating useful error/warning messages like "don't mix these flags - that's not supporter" would be a good thing) > IMO can be reused between Flang and Clang Are there any plans to extract that logic and share it somewhere? > I don't know if Nvidia also want to reuse their toolchain between Clang and > Flang to fully support OpenMP offloading. Who could be the right person to ask? https://github.com/llvm/llvm-project/pull/96742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang-new][OpenMP] Add offload related flags for AMDGPU (PR #96742)
@@ -333,6 +333,9 @@ void Flang::AddAMDGPUTargetArgs(const ArgList &Args, StringRef Val = A->getValue(); CmdArgs.push_back(Args.MakeArgString("-mcode-object-version=" + Val)); } + + const ToolChain &TC = getToolChain(); + TC.addClangTargetOptions(Args, CmdArgs, Action::OffloadKind::OFK_OpenMP); banach-space wrote: +1 to having better diagnostics https://github.com/llvm/llvm-project/pull/96742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang-new][OpenMP] Add offload related flags for AMDGPU (PR #96742)
https://github.com/banach-space edited https://github.com/llvm/llvm-project/pull/96742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang-new][OpenMP] Add offload related flags for AMDGPU (PR #96742)
https://github.com/banach-space commented: > fcuda-is-device flag is not used by Flang currently. In the future it will be > needed for Flang equivalent functions: > AMDGPUTargetCodeGenInfo::getGlobalVarAddressSpace > AMDGPUTargetInfo::getTargetDefines . I don't follow - why would anything related to CUDA be relevant here? https://github.com/llvm/llvm-project/pull/96742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] Revert "[flang] Add basic -mtune support" (PR #96678)
https://github.com/banach-space approved this pull request. https://github.com/llvm/llvm-project/pull/96678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Add basic -mtune support (PR #95043)
banach-space wrote: > LLVM Buildbot has detected a new failure on builder `flang-aarch64-libcxx` > running on `linaro-flang-aarch64-libcxx` while building `clang,flang,mlir` at > step 6 "test-build-unified-tree-check-flang". > > Full details are available at: > https://lab.llvm.org/buildbot/#/builders/89/builds/791 > > Here is the relevant piece of the build log for the reference: > > ``` > Step 6 (test-build-unified-tree-check-flang) failure: test (failure) > TEST 'Flang :: Lower/tune-cpu-llvm.f90' FAILED > > Exit Code: 2 > > Command Output (stderr): > -- > RUN: at line 1: > /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/bin/flang-new > -mtune=pentium4 -S -emit-llvm > /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Lower/tune-cpu-llvm.f90 > -o - | /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/bin/FileCheck > /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Lower/tune-cpu-llvm.f90 > + /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/bin/flang-new > -mtune=pentium4 -S -emit-llvm > /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Lower/tune-cpu-llvm.f90 > -o - > + /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/bin/FileCheck > /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Lower/tune-cpu-llvm.f90 > flang-new: error: unsupported argument 'pentium4' to option '-mtune=' > FileCheck error: '' is empty. > FileCheck command line: > /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/bin/FileCheck > /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Lower/tune-cpu-llvm.f90 > > -- > > > ``` Most likely https://github.com/llvm/llvm-project/pull/95043/files#diff-a29a79ef4763ed66987d979a7a8a4ff87d242101fe133d5188577b9ff144b805 requires X86 to be enabled. Could somebody either fix or revert? I don’t have access to Git ATM. https://github.com/llvm/llvm-project/pull/95043 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Add basic -mtune support (PR #95043)
https://github.com/banach-space closed https://github.com/llvm/llvm-project/pull/95043 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Add basic -mtune support (PR #95043)
banach-space wrote: > if I do so should I also move the target-features-*.f90 tests? Yes, but to me that would qualify as an "unrelated change" (i.e. sth for a separate PR, no need to worry about it here). In general, this PR is about enabling a flag in Flang and making sure that it actually works. This kind of "integration" is a driver task, hence my suggestion. https://github.com/llvm/llvm-project/pull/95043 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [llvm] Remove the Legacy PM Hello example (PR #95708)
https://github.com/banach-space closed https://github.com/llvm/llvm-project/pull/95708 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [llvm] Remove the Legacy PM Hello example (PR #95708)
https://github.com/banach-space edited https://github.com/llvm/llvm-project/pull/95708 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Add basic -mtune support (PR #95043)
https://github.com/banach-space approved this pull request. LGTM, thanks for implementing this 🙏🏻 https://github.com/llvm/llvm-project/pull/95043 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Add basic -mtune support (PR #95043)
banach-space wrote: This is probably a border-line case, but I would consider this a "driver" rather than a "lowering" test. I'm biased though 😅 https://github.com/llvm/llvm-project/pull/95043 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [llvm] Remove the Legacy PM Hello example (PR #95708)
banach-space wrote: > You also need to remove the use of this pass (for testing purposes) from > clang-tools-extra. Done, thanks! I don't understand why CI still fails: ```bash Total Discovered Tests: 60135 -- | Skipped :15 (0.02%) | Unsupported : 1005 (1.67%) | Passed : 58946 (98.02%) | Expectedly Failed: 169 (0.28%) | ninja: build stopped: cannot make progress due to previous errors. | + show-stats | + mkdir -p artifacts | + ccache --print-stats | 🚨 Error: The command exited with status 1 ``` Can you see the actual failure? Looks like all tests passed, right? https://github.com/llvm/llvm-project/pull/95708 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [llvm] Remove the Legacy PM Hello example (PR #95708)
https://github.com/banach-space updated https://github.com/llvm/llvm-project/pull/95708 From d75a05030447e8bcb1dd0b575ff5e7aa5c89f0bb Mon Sep 17 00:00:00 2001 From: Andrzej Warzynski Date: Sun, 16 Jun 2024 13:58:41 +0100 Subject: [PATCH 1/3] [llvm] Remove the Legacy PM Hello example The Legacy PM was deprecated for the optimization pipeline in LLVM 14 [1] (the support was removed altogether in the following release). This patch removes the original Hello example that was introduced to illustrate the Legacy PM. The Hello example no longer works and hence is deleted. The corresponding documentation is also removed. Note, Hello example for new PM is located in * llvm/lib/Transforms/Utils/HelloWorld.cpp and documented in * WritingAnLLVMNewPMPass.rst. [1] https://releases.llvm.org/14.0.0/docs/ReleaseNotes.html#changes-to-the-llvm-ir --- llvm/lib/Transforms/CMakeLists.txt | 1 - llvm/lib/Transforms/Hello/CMakeLists.txt | 20 llvm/lib/Transforms/Hello/Hello.cpp | 64 llvm/lib/Transforms/Hello/Hello.exports | 0 4 files changed, 85 deletions(-) delete mode 100644 llvm/lib/Transforms/Hello/CMakeLists.txt delete mode 100644 llvm/lib/Transforms/Hello/Hello.cpp delete mode 100644 llvm/lib/Transforms/Hello/Hello.exports diff --git a/llvm/lib/Transforms/CMakeLists.txt b/llvm/lib/Transforms/CMakeLists.txt index 84a7e34147d08..7046f2f4b1d2c 100644 --- a/llvm/lib/Transforms/CMakeLists.txt +++ b/llvm/lib/Transforms/CMakeLists.txt @@ -5,7 +5,6 @@ add_subdirectory(InstCombine) add_subdirectory(Scalar) add_subdirectory(IPO) add_subdirectory(Vectorize) -add_subdirectory(Hello) add_subdirectory(ObjCARC) add_subdirectory(Coroutines) add_subdirectory(CFGuard) diff --git a/llvm/lib/Transforms/Hello/CMakeLists.txt b/llvm/lib/Transforms/Hello/CMakeLists.txt deleted file mode 100644 index 9510c31f633fe..0 --- a/llvm/lib/Transforms/Hello/CMakeLists.txt +++ /dev/null @@ -1,20 +0,0 @@ -# If we don't need RTTI or EH, there's no reason to export anything -# from the hello plugin. -if( NOT LLVM_REQUIRES_RTTI ) - if( NOT LLVM_REQUIRES_EH ) -set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/Hello.exports) - endif() -endif() - -if(WIN32 OR CYGWIN OR ZOS) - set(LLVM_LINK_COMPONENTS Core Support) -endif() - -add_llvm_library( LLVMHello MODULE BUILDTREE_ONLY - Hello.cpp - - DEPENDS - intrinsics_gen - PLUGIN_TOOL - opt - ) diff --git a/llvm/lib/Transforms/Hello/Hello.cpp b/llvm/lib/Transforms/Hello/Hello.cpp deleted file mode 100644 index b0adb5401f891..0 --- a/llvm/lib/Transforms/Hello/Hello.cpp +++ /dev/null @@ -1,64 +0,0 @@ -//===- Hello.cpp - Example code from "Writing an LLVM Pass" ---===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===--===// -// -// This file implements two versions of the LLVM "Hello World" pass described -// in docs/WritingAnLLVMPass.html -// -//===--===// - -#include "llvm/ADT/Statistic.h" -#include "llvm/IR/Function.h" -#include "llvm/Pass.h" -#include "llvm/Support/raw_ostream.h" -using namespace llvm; - -#define DEBUG_TYPE "hello" - -STATISTIC(HelloCounter, "Counts number of functions greeted"); - -namespace { - // Hello - The first implementation, without getAnalysisUsage. - struct Hello : public FunctionPass { -static char ID; // Pass identification, replacement for typeid -Hello() : FunctionPass(ID) {} - -bool runOnFunction(Function &F) override { - ++HelloCounter; - errs() << "Hello: "; - errs().write_escaped(F.getName()) << '\n'; - return false; -} - }; -} - -char Hello::ID = 0; -static RegisterPass X("hello", "Hello World Pass"); - -namespace { - // Hello2 - The second implementation with getAnalysisUsage implemented. - struct Hello2 : public FunctionPass { -static char ID; // Pass identification, replacement for typeid -Hello2() : FunctionPass(ID) {} - -bool runOnFunction(Function &F) override { - ++HelloCounter; - errs() << "Hello: "; - errs().write_escaped(F.getName()) << '\n'; - return false; -} - -// We don't modify the program, so we preserve all analyses. -void getAnalysisUsage(AnalysisUsage &AU) const override { - AU.setPreservesAll(); -} - }; -} - -char Hello2::ID = 0; -static RegisterPass -Y("hello2", "Hello World Pass (with getAnalysisUsage implemented)"); diff --git a/llvm/lib/Transforms/Hello/Hello.exports b/llvm/lib/Transforms/Hello/Hello.exports deleted file mode 100644 index e69de29bb2d1d..0 From bed0a86befc6f6b88a28a0eec53499e02b8bddad Mon Sep 17 00:00:00 2001 From: Andrzej Warzynski Date: Sun, 16 Jun 2024 17:10:37 +0100 Subject: [PATCH 2/3] fixup! [llvm]
[clang] [flang] [flang] Implement -mcmodel flag (PR #95411)
@@ -0,0 +1,16 @@ +! RUN: %flang_fc1 -triple aarch64 -emit-llvm -mcmodel=tiny %s -o - | FileCheck %s -check-prefix=CHECK-TINY banach-space wrote: [nit] IMHO, using `CHECK` for prefixes in a test with multiple prefixes is just noise. Why not drop `CHECK`? https://github.com/llvm/llvm-project/pull/95411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Implement -mcmodel flag (PR #95411)
https://github.com/banach-space approved this pull request. The high-level stuff makes sense to me, thanks! I'm not familiar with `-mcmodel`, so can't tell for sure whether the tests are correct 😅 Ideally one more person would take a look - @tblah or @pawosm-arm ? https://github.com/llvm/llvm-project/pull/95411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Implement -mcmodel flag (PR #95411)
https://github.com/banach-space edited https://github.com/llvm/llvm-project/pull/95411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Implement -mcmodel flag (PR #95411)
@@ -2823,3 +2823,84 @@ void tools::addOffloadCompressArgs(const llvm::opt::ArgList &TCArgs, CmdArgs.push_back( TCArgs.MakeArgString(Twine("-compression-level=") + Arg->getValue())); } + +void tools::addMCModel(const Driver &D, const llvm::opt::ArgList &Args, banach-space wrote: Is this taken verbatim from Clang? Would you mind documenting that in the summary? https://github.com/llvm/llvm-project/pull/95411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Add -mlink-builtin-bitcode option to fc1 (PR #94763)
https://github.com/banach-space approved this pull request. https://github.com/llvm/llvm-project/pull/94763 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Add basic -mtune support (PR #95043)
@@ -32,6 +32,9 @@ class TargetOptions { /// If given, the name of the target CPU to generate code for. std::string cpu; + /// If given, the name of the target CPU to tune code for. + std::string tuneCPU; banach-space wrote: I'm fine with longer self-documenting names, e.g. `cpuToTuneFor`. That would be more consistent with `cpu` above :) https://github.com/llvm/llvm-project/pull/95043 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Add basic -mtune support (PR #95043)
https://github.com/banach-space commented: Thanks for working on this @AlexisPerry ! Could you add some tests? https://github.com/llvm/llvm-project/pull/95043 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [mlir] [flang] Add basic -mtune support (PR #95043)
https://github.com/banach-space edited https://github.com/llvm/llvm-project/pull/95043 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Add -mlink-builtin-bitcode option to fc1 (PR #94763)
@@ -1146,6 +1150,54 @@ void CodeGenAction::embedOffloadObjects() { } } +void CodeGenAction::linkBuiltinBCLibs() { banach-space wrote: +1 https://github.com/llvm/llvm-project/pull/94763 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Add -mlink-builtin-bitcode option to fc1 (PR #94763)
@@ -0,0 +1,18 @@ + +!-- +! RUN lines +!-- +! Embed something that can be easily checked +! RUN: %flang_fc1 -emit-llvm -triple x86_64-unknown-linux-gnu -o - -mlink-builtin-bitcode %S/Inputs/bclib.bc %s 2>&1 | FileCheck %s + +! CHECK: define internal void @libfun_ + +! RUN1: not %flang_fc1 -emit-llvm -triple x86_64-unknown-linux-gnu -o - -mlink-builtin-bitcode %S/Inputs/no-bclib.bc %s 2>&1 | FileCheck %s + +! ERROR1: error: could not open {{.*}} no-bclib.bc banach-space wrote: What's ERROR1? https://github.com/llvm/llvm-project/pull/94763 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Add -mlink-builtin-bitcode option to fc1 (PR #94763)
@@ -0,0 +1,18 @@ + +!-- +! RUN lines +!-- +! Embed something that can be easily checked +! RUN: %flang_fc1 -emit-llvm -triple x86_64-unknown-linux-gnu -o - -mlink-builtin-bitcode %S/Inputs/bclib.bc %s 2>&1 | FileCheck %s banach-space wrote: 1. What's bclib.bc and what makes it easy to check? 2. Does the triple matter? https://github.com/llvm/llvm-project/pull/94763 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Add -mlink-builtin-bitcode option to fc1 (PR #94763)
@@ -0,0 +1,18 @@ + +!-- +! RUN lines +!-- banach-space wrote: In the past, folks asked not to add such comments in tests. Let's stick with that. https://github.com/llvm/llvm-project/pull/94763 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Add -mlink-builtin-bitcode option to fc1 (PR #94763)
@@ -0,0 +1,18 @@ + +!-- +! RUN lines +!-- +! Embed something that can be easily checked +! RUN: %flang_fc1 -emit-llvm -triple x86_64-unknown-linux-gnu -o - -mlink-builtin-bitcode %S/Inputs/bclib.bc %s 2>&1 | FileCheck %s + +! CHECK: define internal void @libfun_ + +! RUN1: not %flang_fc1 -emit-llvm -triple x86_64-unknown-linux-gnu -o - -mlink-builtin-bitcode %S/Inputs/no-bclib.bc %s 2>&1 | FileCheck %s banach-space wrote: What's RUN1? https://github.com/llvm/llvm-project/pull/94763 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][OpenMP] Add -fopenmp-force-usm option to flang (PR #94359)
https://github.com/banach-space approved this pull request. https://github.com/llvm/llvm-project/pull/94359 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86] Support EGPR for inline assembly. (PR #92338)
banach-space wrote: Should be fixed by https://github.com/llvm/llvm-project/pull/93794 https://github.com/llvm/llvm-project/pull/92338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86] Support EGPR for inline assembly. (PR #92338)
banach-space wrote: Hi, thanks for this contribution. Sadly, it messes up my local checkout on MacOS (which is insensitive when it comes to files names). These files are problematic: * "asm-constraint-jR.ll" and "asm-constraint-jr.ll" Please, could you rename them so that they are not identical on case-insensitive systems? https://github.com/llvm/llvm-project/pull/92338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][flang][windows] Prefer user-provided library paths (-L) (PR #90758)
banach-space wrote: > I've left the flang test as the flang directory doesn't change with > `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` and removed the other test. I hope this > is what you meant @MaskRay 👍 That's matches how I read that suggestion re `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR`, thanks! From what I can tell, you've already made all the changes requested from @MaskRay . If there are no new comments today, I suggest landing this. https://github.com/llvm/llvm-project/pull/90758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] New -fdebug-unparse-with-modules option (PR #91660)
https://github.com/banach-space approved this pull request. LGTM, thanks! https://github.com/llvm/llvm-project/pull/91660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Accept -fallow-argument-mismatch with a warning instead of rejecting it (PR #91611)
banach-space wrote: Thank you for this contribution Paul! > Many autotools-utilizing projects (mpich among them) fail to complete the > configure step since it tries to invoke the (unknown to them) Fortran > compiler always with the -fallow-argument-mismatch flag. It sounds like an issue with the `mpich` build script rather than LLVM Flang. Why would `mpich` and other projects use this flag to begin with? Do other Fortran compilers support it? From GFrotran docs (https://gcc.gnu.org/onlinedocs/gfortran/Fortran-Dialect-Options.html): > Using this option is strongly discouraged. In general, I'm hesitant to allow such dummy flags that: * do nothing, * are there merely to prevent issues in build scripts. Are there no other ways around it? We obviously should make it possible for LLVM Flang to build all projects, but I really want to avoid Flang containing multiple dummy flags only to work around build scripts that assume a particular compiler is used. We might be missing some "gfortran compatibility mode" in Flang (on top of `FlangMode`). Such mode could allow unsupported GFortran flags and warn whenever those are used. `flang-new` would still "default" to `FlangMode` and just generate an error. In any case, it would be good to get more opinions on this. @kiranchandramohan , anyone else might be interested in this? https://github.com/llvm/llvm-project/pull/91611 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang] RFC: Add support for -w option 1/n (PR #90420)
banach-space wrote: [nit] Once you have more than one prefix, `CHECK` becomes noise. At least IMHO 😅 (we all know that these are "check" lines). You could use more descriptive prefixes instead, e.g. `CHECK-PORT` -> `PORTABILITY` (it wasn't obvious to me that `PORT` stood for "portability"), `CHECK` -> `DEFAULT`. https://github.com/llvm/llvm-project/pull/90420 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang] RFC: Add support for -w option 1/n (PR #90420)
https://github.com/banach-space approved this pull request. LGTM, thanks! https://github.com/llvm/llvm-project/pull/90420 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang] RFC: Add support for -w option 1/n (PR #90420)
https://github.com/banach-space edited https://github.com/llvm/llvm-project/pull/90420 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Add -print-resource-dir command line flag to emit Flang's resource directory (PR #90886)
https://github.com/banach-space approved this pull request. LGTM, thanks for working on this and for addressing my comments 🙏🏻 https://github.com/llvm/llvm-project/pull/90886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][flang][windows] Prefer user-provided library paths (-L) (PR #90758)
@@ -0,0 +1,8 @@ +! REQUIRES: system-windows +! +! RUN: %clang --driver-mode=flang -### %s -Ltest 2>&1 | FileCheck %s banach-space wrote: [nit] `test` -> `random_test_dir` or something else that will make this stand out a bit more (makes parsing tests a bit easier) https://github.com/llvm/llvm-project/pull/90758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][flang][windows] Prefer user-provided library paths (-L) (PR #90758)
@@ -0,0 +1,8 @@ +! REQUIRES: system-windows +! +! RUN: %clang --driver-mode=flang -### %s -Ltest 2>&1 | FileCheck %s +! +! Test that user provided paths come before the Flang runtimes and compiler-rt +! CHECK: "-libpath:test" banach-space wrote: Wondering how to check that there's no `-libapth` before `-libpath:test` 🤔 https://github.com/llvm/llvm-project/pull/90758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][flang][windows] Prefer user-provided library paths (-L) (PR #90758)
https://github.com/banach-space approved this pull request. LGTM, thanks! I've left some nits, feel free to ignore https://github.com/llvm/llvm-project/pull/90758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][flang][windows] Prefer user-provided library paths (-L) (PR #90758)
https://github.com/banach-space edited https://github.com/llvm/llvm-project/pull/90758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Add -print-resource-dir command line flag to emit Flang's resource directory (PR #90886)
banach-space wrote: > > How does this compare to GFortran and Classic Flang? Anything resembling > > this flag? > > GFortran does not have it, but Classic Flang does. So, it's closing a gap to > Classic Flang here as well. Do you know the meaning for Classic Flang? Would be great to make "LLVM Flang" consistent with any "prior art". Nice to have ;-) https://github.com/llvm/llvm-project/pull/90886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Add -print-resource-dir command line flag to emit Flang's resource directory (PR #90886)
@@ -0,0 +1,3 @@ +! RUN: %flang -print-resource-dir -resource-dir=%S/Inputs/resource_dir \ +! RUN: | FileCheck -check-prefix=PRINT-RESOURCE-DIR -DFILE=%S/Inputs/resource_dir %s banach-space wrote: > So, my feeling is that this refactoring should take place in a different PR. Makes sense. https://github.com/llvm/llvm-project/pull/90886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Add -print-resource-dir command line flag to emit Flang's resource directory (PR #90886)
@@ -5474,7 +5474,7 @@ def print_prog_name_EQ : Joined<["-", "--"], "print-prog-name=">, Visibility<[ClangOption, CLOption]>; def print_resource_dir : Flag<["-", "--"], "print-resource-dir">, HelpText<"Print the resource directory pathname">, - Visibility<[ClangOption, CLOption]>; + Visibility<[ClangOption, CLOption, FlangOption]>; banach-space wrote: > Do you suggest that the help message needs to be more verbose for Flang? Yes. Why not make it more user-friendly? It's a nice to have though. > Print the resource directory pathname where Flang stores the 'lib' and > 'include' folder Looking at your reply earlier: > I'd like to at least have it point to where the MODULE files live. Shouldn't help text mention MODULEs? https://github.com/llvm/llvm-project/pull/90886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Add -print-resource-dir command line flag to emit Flang's resource directory (PR #90886)
@@ -5474,7 +5474,7 @@ def print_prog_name_EQ : Joined<["-", "--"], "print-prog-name=">, Visibility<[ClangOption, CLOption]>; def print_resource_dir : Flag<["-", "--"], "print-resource-dir">, HelpText<"Print the resource directory pathname">, - Visibility<[ClangOption, CLOption]>; + Visibility<[ClangOption, CLOption, FlangOption]>; banach-space wrote: You can use `HelpVariant` to and help text specific to Flang. Example: * https://github.com/llvm/llvm-project/blob/6d44a1ef55b559e59d725b07ffe1da988b4e5f1c/clang/include/clang/Driver/Options.td#L813-L815 https://github.com/llvm/llvm-project/pull/90886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Add -print-resource-dir command line flag to emit Flang's resource directory (PR #90886)
@@ -0,0 +1,3 @@ +! RUN: %flang -print-resource-dir -resource-dir=%S/Inputs/resource_dir \ +! RUN: | FileCheck -check-prefix=PRINT-RESOURCE-DIR -DFILE=%S/Inputs/resource_dir %s +! PRINT-RESOURCE-DIR: [[FILE]] banach-space wrote: I have a suspicion that this would also print clang-specific stuff? https://github.com/llvm/llvm-project/pull/90886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Add -print-resource-dir command line flag to emit Flang's resource directory (PR #90886)
@@ -0,0 +1,3 @@ +! RUN: %flang -print-resource-dir -resource-dir=%S/Inputs/resource_dir \ +! RUN: | FileCheck -check-prefix=PRINT-RESOURCE-DIR -DFILE=%S/Inputs/resource_dir %s banach-space wrote: You should be able to avoid repeating "%S/Inputs/resource_dir" if you use `define`/`redefine`: * example: https://github.com/llvm/llvm-project/blob/6d44a1ef55b559e59d725b07ffe1da988b4e5f1c/mlir/test/Integration/Dialect/Linalg/CPU/ArmSVE/matmul.mlir#L1-L14 * docs: https://llvm.org/docs/TestingGuide.html (search for `DEFINE`) https://github.com/llvm/llvm-project/pull/90886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Add -print-resource-dir command line flag to emit Flang's resource directory (PR #90886)
@@ -250,6 +247,25 @@ void Driver::setDriverMode(StringRef Value) { Diag(diag::err_drv_unsupported_option_argument) << OptName << Value; } +void Driver::setResourceDirectory() { + // Compute the path to the resource directory, depending on the driver mode. + switch (Mode) { + case GCCMode: + case GXXMode: + case CPPMode: + case CLMode: + case DXCMode: +ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR); +break; + case FlangMode: +// TODO: Is there a better way to add the "../include/flang/" component? +SmallString<64> relPath{}; +llvm::sys::path::append(relPath, "..", "include", "flang"); +ResourceDir = GetResourcesPath(ClangExecutable, relPath); +break; banach-space wrote: Presumably `relPath` stands for "relative path"? Why not `customResoursePath` as in https://github.com/llvm/llvm-project/blob/6d44a1ef55b559e59d725b07ffe1da988b4e5f1c/clang/lib/Driver/Driver.cpp#L166-L192 ? Or, probably more accurate, `customResourcePathReleativeToDriver` (long names are fine with me). Wouldn't the above also add things that are only relevant for Clang? As in, shouldn't `GetResourcesPath` be specialised for Flang? Btw, where is `ClangExecutable` defined? In ideal world it would be something more generic, e.g. `DriverExecutable`. But I appreciate that that's tangential to this change :) https://github.com/llvm/llvm-project/pull/90886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Add -print-resource-dir command line flag to emit Flang's resource directory (PR #90886)
banach-space wrote: > > What's the definition of "resource dir" for Fortran? > > I'd like to at least have it point to where the MODULE files live. > > When I look at what clang emits, then Flang's resource directory should > rather point to the place, where Flang has its `lib` and `include` > directories. So, one level above of what I have done in this PR right now. > How does this compare to GFortran and Classic Flang? Anything resembling this flag? https://github.com/llvm/llvm-project/pull/90886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [Flang][Driver] Add -print-resource-dir command line flag to emit Flang's resource directory (PR #90886)
https://github.com/banach-space commented: What's the definition of "resource dir" for Fortran? https://github.com/llvm/llvm-project/pull/90886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [lld] [flang] Generate main only when a Fortran program statement is present (PR #89938)
banach-space wrote: Great work David, thanks! Could you add some documentation explaining _where_ `main` would be coming from in the case of mixed-source compilation? In fact, is that tested anywhere? Also, IMHO it would be good to advertise this on Discourse (thinking specifically about `-fno-fortran-main`). https://github.com/llvm/llvm-project/pull/89938 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Avoid mentions of Clang in Flang's command line reference. (PR #88932)
https://github.com/banach-space approved this pull request. https://github.com/llvm/llvm-project/pull/88932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Avoid mentions of Clang in Flang's command line reference. (PR #88932)
banach-space wrote: > Would you like me to introduce DocBriefForVariants? +1 That would be helpful for `-I`: * https://flang.llvm.org/docs/FlangCommandLineReference.html#cmdoption-flang-I-dir Ideally we'd find more examples (so that you are not adding it for just one option). As for this: ``` def Diag_Group : OptionGroup<"">, Group, DocName<"Diagnostic options">, DocBrief.str, !cond(!eq(GlobalDocumentation.Program, "Clang"): "See the :doc:`full list of warning and remark flags `.", true: "See Clang's Diagnostic Reference for a full list of warning and remark flags." ) )>; ``` Wouldn't replacing `Clang's` with `%Program's` be sufficient in this case? https://github.com/llvm/llvm-project/pull/88932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Avoid mentions of Clang in Flang's command line reference. (PR #88932)
banach-space wrote: > > Clang is also mentioned for the diagnostic warnings reference, which mostly > > applies to C/C++/Obj-C, not Fortran. #81726 already tried to fix this, and > > I don't know a better solution. > > Do you mean that this PR fixes this, or that you noticed this problem while > working on this? > > If it's the latter it may be as @banach-space mentioned to me, that the > `docbrief` is still shared between Flang and Clang. This could be solved in a > similar, albeit tedious way to what I did for the help text. @Meinersbur Could you share an example? If it's a problem specific to diagnostics then we should possibly just start separating Clang and Flang diagnostics. That could mean a bit of duplication, but not too much, so I'm not worried. https://github.com/llvm/llvm-project/pull/88932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][driver] Avoid mentions of Clang in Flang's command line reference. (PR #88932)
https://github.com/banach-space approved this pull request. Nice, thank you! LGTM https://github.com/llvm/llvm-project/pull/88932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][cuda] Enable cuda with -x cuda option (PR #84944)
https://github.com/banach-space approved this pull request. LGTM, thanks for the updates! https://github.com/llvm/llvm-project/pull/84944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][cuda] Add -fcuda option (PR #84944)
@@ -0,0 +1,13 @@ +! Test -fcuda option +! RUN: %flang -fc1 -cpp -fcuda -fdebug-unparse %s -o - | FileCheck %s banach-space wrote: Could you add a RUN line without enabling CUDA? Otherwise it's hard to see what's being tested and what the impact of enabling CUDA is. https://github.com/llvm/llvm-project/pull/84944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][cuda] Add -fcuda option (PR #84944)
@@ -6488,6 +6488,9 @@ defm stack_arrays : BoolOptionWithoutMarshalling<"f", "stack-arrays", defm loop_versioning : BoolOptionWithoutMarshalling<"f", "version-loops-for-stride", PosFlag, NegFlag>; + +def fcuda : Flag<["-"], "fcuda">, Group, banach-space wrote: I don't quite understand what actually `-fcuda` enables. Could the flag be more descriptive and the help text expanded? Also, what's the equivalent in Clang? We ought to keep both drivers in sync. In particular, if this is something specific to Flang then I would avoid generic names like `-fcuda`. https://github.com/llvm/llvm-project/pull/84944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits