[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
nikic wrote: As this is time critical (needs to land prior to the LLVM 21 release tomorrow) I've submitted a PR myself: https://github.com/llvm/llvm-project/pull/155279 https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
nikic wrote: @sjoerdmeijer Let me address two points separately. The first is the original submission of this PR. There was an existing PR to enable loop interchange in https://github.com/llvm/llvm-project/pull/124911, and discussion for this enablement was consolidated there. I can see no evidence that reviewers at the time believed that loop interchange / dependence analysis is ready to be enabled. You *could* have posted a comment there asking whether people think it's okay to enable it just for Flang due to difference language characteristics -- and I expect you'd have gotten a fairly clear "no" on making this language-dependent. But that did not happen. I guess this was just a miscommunication. Now, regarding the issues in dependence analysis that have been found more recently: Yes, I believe these issues are quite severe and justify a revert. It's not that these issues are easy to trigger, but that they point to some rather fundamental issues in the dependence analysis implementation, which will likely require non-trivial changes to fully address. I don't want to backport all necessary changes to LLVM 21. And yes, there is a certain double standard when it comes to issues in a newly enabled pass, and issues in a pass that has already been enabled for a very long time. In the latter case, it would take some rather extreme circumstances for us to disable the pass entirely, while in the former case this is the default response for non-trivial issues. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
sjoerdmeijer wrote: @nikic: I feel that all nuance has been lost here in this discussion, so I would like to bring some of that back: - First of all, let's recognise that Fortran and C/C++ are really different, and that they are different users of DA. At the time of enablement here, the last known bug that is being worked on involved type-punning of base pointers in a loop body, and/or some variants of this. You can't write that in Fortran. And in C/C++ you won't find this in normal code (it's also non-portable code). - In a bug triage process, before any action is taken, first the trigger conditions should be determined, and then the impact to determine the severity. We don't have any of this information. I thus feel we have different standards for DA compared to other components that have problems (that may or may not have an impact). We should also recognise that new information appeared after this patch was merged, and very recently. I - I haven't looked at all new bugs, but one of them is definitely another very weird corner case that I doubt can be triggered from Fortran. And whether the exact process has been followed for enablement I don't know, but now the fact is that this has been running for 3 months and no bugs have been raised against Flang or interchange. If you feel this should be reverted based on the grounds of process or inclusion, okay, then it is what it is, fair enough. Going forward though I would encourage a more constructive approach to deal with bugs before any conclusions are drawn. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
kasuga-fj wrote: I’ll leave the revert up to the author. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
kasuga-fj wrote: > To be honest, I am very surprised that this PR was submitted, let alone > landed. We have **explicitly not approved** enabling LoopInterchange by > default in LLVM due to outstanding issues. So instead you go ahead and enable > it directly in Flang instead, without even notifying any of the people who > were involved in the original discussions. That's not how things should work. > > Please revert this patch ASAP and request an LLVM 21 backport. (Thank you very much for finding this PR...) https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
nikic wrote: To be honest, I am very surprised that this PR was submitted, let alone landed. We have **explicitly not approved** enabling LoopInterchange by default in LLVM due to outstanding issues. So instead you go ahead and enable it directly in Flang instead, without even notifying any of the people who were involved in the original discussions. That's not how things should work. Please revert this patch ASAP and request an LLVM 21 backport. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
sebpop wrote: > I'm no longer sure what's realistic TBH, nobody writes such code: `a[j & 1][i & 1]`. Anyway, we have a fix for all those. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
sebpop wrote: I posted a patch that fixes your above testcase. I also added the above testcase as [llvm/test/Analysis/DependenceAnalysis/wrapping-addrec-1.ll](https://github.com/llvm/llvm-project/commit/61ebb6856c54b4c286a45bc8cda2ae926b120349#diff-e6b4c7d37f82576f8517137c898258a2463cc910b8d025bf258afae700fd885c) The output is ``` ; CHECK-NEXT:da analyze - output [* *]! ``` https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
sebpop wrote: Instead of spreading FUD around, please be more precise and open bug reports for the following: > There are fundamental flaws in DA's implementation, particularly in how it > handles wrapping of SCEVs. Tell me more about it in a separate bug, and let's see how we can work towards fixing it. > These issues aren't caught by assertions and can silently lead to incorrect > transformations. What are you speaking about? Again this is FUD. > I don't know how many miscompiles exist, but it would not be a small number. Please provide data. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
Meinersbur wrote: > > There are fundamental flaws in DA's implementation, particularly in how it > > handles wrapping of SCEVs. > > Tell me more about it in a separate bug, and let's see how we can work > towards fixing it. If you look over the code you can see that no attention was paid to integer wrapping behavior. For instance, `x < y` is implemented in multiple places (e.g. `DependenceInfo::isKnownLessThan`) as ``` SE->isKnownNegative(getMinusSCEV(x,y)) ``` 1. `isKnownNegative` always assumes an signed interpretation (otherwise it could never be negative), but the code may use unsigned loop variables/pointer indices 2. Even if it is signed, if eg `x = 1`, `y = INT_MIN` (so x is obviously larger) `x - y` would wrap back into the negative and return true. > Instead of spreading FUD around [...] @kasuga-fj pointed out bugs in the current implementation. Whether those are sufficient to justify changing default behavior is subjective. Generally assume best intentions. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
sebpop wrote: > Generally assume best intentions. I agree, and I will keep this in mind. > These issues aren't caught by assertions and can silently lead to incorrect > transformations. For GCC, I adapted the Omega test (from Bill Pugh) to produce the same representation as the classic dependence tests. And we got more confidence that the dependence analysis implementation was correct. I removed the extra checks https://github.com/gcc-mirror/gcc/commit/49b8fe6c1a585edfeb5dd0f292e05a167f475f68 10 years down the road. Maybe we could do the same to build trust in the results provided by LLVM's DA. We could adapt Polly to print distance and direction vectors and then diff DA against Polly. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
sebpop wrote: Thank you for raising these issues to my attention, I will be working on fixing these bugs. I believe those bugs may not impact Fortran programs as generated by flang: 1. One data point is our internal runs of SPEC Cpu benchmarks on which interchange does trigger with flang, and there are no correctness issues. 2. Another data point is your earlier comment: > I've confirmed the result of Fujitsu Compiler Test Suite. The only > correctness issue affected by this commit is > https://github.com/fujitsu/compiler-test-suite/blob/main/Fortran/0347/0347_0240.f, > which will be resolved by https://github.com/llvm/llvm-project/pull/140709. 3. So far I have not seen bugs reported against flang + interchange. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
tblah wrote: Weighing in with my personal opinion here: - If this can be shown to miss compile any known application then that's grounds for imediately disabling the pass until it can be fixed. - If this only breaks some very obscure fortran formulation that it is unlikely for a human to write then I guess we should weigh up pros and cons. I would lean towards a revert but there have been exceptions made in the past for critical optimisations which are not known to break on any known production code (the example I'm thinking of was a theoretical correctness issue with the TBAA alias tags when a function is inlined into itself which we were confident didn't effect real code because classic Flang had the same issue). - If this is unlikely to be triggered from any fortran code and the pass is providing a noticeable speedup such that disabling it would constitute a measurable regression in Flang codegen then we may decide not to disable the pass, especially if DA is going to be fixed relatively soon. In this particular case, the pass has been merged for a long time. In that time we have built a lot of applications and benchmarks at -O3 in our internal CI and not seen any issues known to arise from loop interchange. Presumably other organisations have been testing Flang during this time too. @kasuga-fj thank you very much for reporting this. It is helpful for the whole community to know that there could be issues here so that we know where to look if any miss-compilations are discovered. Your careful review of DA looks like a great step forward. If the reported issues break something you are doing with Flang then I will be happy to consider disabling the pass. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
kasuga-fj wrote: > This is wrong, we fixed all outstanding bugs reported against DA at the time > when we enabled interchange in flang. I’m referring to the bugs that existed at that time but were not reported. The issues that were raised do not represent all the defects. > If we don't enable interchange by default then we don't get more bugs > reported against it because the pass is not getting used. I don’t think enabling passes just to get more bug reports is a good strategy. At the very least, we should carefully check the existing code beforehand. To be clear: all my bug reports are what I found by reading the code. I didn't use flang to find them. > How many of the current 4 bugs you point to are related to flang? > There is no mention about flang in any of the bug reports. Are there any guarantees that Flang won’t generate the IR I mentioned in the issues? If so, that’s fine, but I don’t think any such guarantees exist. > Instead of spreading FUD around, please be more precise and open bug reports > for the following: At the very least I can’t help feeling anxious, since there seem to be so many issues that it’s really hard to imagine what might happen. > Tell me more about it in a separate bug, and let's see how we can work > towards fixing it. Here are some examples that immediately come to my mind (not limited to them): - [This](https://github.com/llvm/llvm-project/blob/a53e73e6efb4cfe0440b7b35496c0c72b3ae6c4f/llvm/lib/Analysis/DependenceAnalysis.cpp#L1246-L1249) is wrong, `!isKnownNegative` doesn't mean it's positive. - [This](https://github.com/llvm/llvm-project/blob/a53e73e6efb4cfe0440b7b35496c0c72b3ae6c4f/llvm/lib/Analysis/DependenceAnalysis.cpp#L1250) seems incorrect, since we don't check the monotonicity of each subscript. - [This](https://github.com/llvm/llvm-project/blob/a53e73e6efb4cfe0440b7b35496c0c72b3ae6c4f/llvm/lib/Analysis/DependenceAnalysis.cpp#L1284-L1286) is wrong, we don't check whether `X` is non-zero. - [This](https://github.com/llvm/llvm-project/blob/a53e73e6efb4cfe0440b7b35496c0c72b3ae6c4f/llvm/lib/Analysis/DependenceAnalysis.cpp#L3116) is wrong. In general, the no-wrap flags aren't preserved. - [In this function](https://github.com/llvm/llvm-project/blob/a53e73e6efb4cfe0440b7b35496c0c72b3ae6c4f/llvm/lib/Analysis/DependenceAnalysis.cpp#L2123), we must ensure the multiplication doesn't overflow. But the most serious issue is that DA doesn’t handle overflow at all. > > These issues aren't caught by assertions and can silently lead to incorrect > > transformations. > > What are you speaking about? I say that DA can miss dependency, and it can result in incorrect legality check in LoopInterchange. > Please provide data. I don’t have any data, so I'm fine if the Flang community is okay with it. However, I strongly think these issues also affect Flang. In the end, there’s no data showing that these issues are unrelated to Flang. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
sebpop wrote: > DA was already broken when this PR was submitted. This is wrong, we fixed all outstanding bugs reported against DA at the time when we enabled interchange in flang. > I believe it's premature to enable any passes that rely on DA by default. If we don't enable interchange by default then we don't get more bugs reported against it because the pass is not getting used. > This would not directly reflect the severity of the issue, but it's the > conclusion I reached after thoroughly examining DA for a month. > There are simply too many bugs. *too many* is not substantiated. Please provide a list of bugs reported against flang due to interchange. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
sjoerdmeijer wrote: > There are fundamental flaws in DA's implementation, particularly in how it > handles wrapping of SCEVs. So the real question here is: how important or realistic is this for Fortran? https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
kasuga-fj wrote: I'm not trying to emphasize the number of issues raised on GitHub, the real concern is their substance. What I pointed out in the issues represents only a small portion of the overall problem. There are fundamental flaws in DA's implementation, particularly in how it handles wrapping of SCEVs. These issues aren't caught by assertions and can silently lead to incorrect transformations. I don't know how many miscompiles exist, but it would not be a small number. The situation seems to differ from that of the vectorizer. DA was already broken when this PR was submitted. I believe it's premature to enable any passes that rely on DA by default. This would not directly reflect the severity of the issue, but it's the conclusion I reached after thoroughly examining DA for a month. There are simply too many bugs. Also note that properly fixing these issues will take considerable time, as we (or at least I) still aren't sure what the best approach to solving them is. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
sjoerdmeijer wrote: > To Flang community: After this PR was merged, [lots of > miscompilations](https://github.com/llvm/llvm-project/issues?q=is%3Aissue%20state%3Aopen%20DA%20label%3Allvm%3Aanalysis%2Ccrash%20created%3A%3E2025-07-01) > were discovered in DependenceAnalysis, which LoopInterchange depends on. > Resolving these issues may take some time, so I suggest we disable > LoopInterchange by default for now. I'm not very familiar with Flang's > policy, so, what does the community think about this? You're pointing to 4 patches, one of them I raised which I found by using different fuzzers. In my fuzzing exercise, I found ~30 issues, and I found a lot more issues in the loop vectoriser than there are issues in DA. Are we going to disable the vectoriser because we raise issues against it of which some of them are still open? That's a rhetorical question. My point is, this needs to be judged on a case by case basis, and we shouldn't just point to a list of less than a handful issues, that is not going to be useful. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
kasuga-fj wrote: It is only enabled in flang, and not in clang at the moment. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
tarunprabhu wrote: Does this affect clang as well? If we do decide to disable it, will it only be in flang, or in both clang and flang? If the latter, we should check with the clang developers as well. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
Meinersbur wrote: Flang's policy is not really different from Clang's, but I can raise it in the next community call https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
kasuga-fj wrote: > Flang's policy is not really different from Clang's, but I can raise it in > the next community call Thanks. I think this should be removed from LLVM 21. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
kawashima-fj wrote: I've confirmed the result of Fujitsu Compiler Test Suite. The only correctness issue affected by this commit is https://github.com/fujitsu/compiler-test-suite/blob/main/Fortran/0347/0347_0240.f, which will be resolved by #140709. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
kasuga-fj wrote: There is still a correctness issue with LoopInterchange, as I reported in #140238. This problem is not specific to C/C++. The following code demonstrates a case where illegal loop-interchange occurs. ```fortran program main implicit none real, save :: A(5, 5) real, target :: V(16), W(16) real, pointer :: X(:, :) => null(), Y(:, :) => null() integer :: i, j do i = 0, 15 A(i / 4 + 1, mod(i, 4) + 1) = real(i) V(i + 1) = real(i + i) W(i + 1) = real(i * i) end do X(1:4, 1:4) => V(:) if (A(2, 2) < A(3, 3)) then Y(1:4, 1:4) => V(:) else Y(1:4, 1:4) => W(:) endif ! Illegal interchange occurs do j = 1, 4 do i = 1, 4 X(i, j) = Y(j, i) A(i + 1, j) = A(i, j) + 1 end do end do print *, X end program main ``` godbolt: https://godbolt.org/z/db5qaab1T This issue should be resolved by #140709. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
@@ -3149,3 +3149,16 @@ void tools::handleVectorizeSLPArgs(const ArgList &Args,
options::OPT_fno_slp_vectorize, EnableSLPVec))
CmdArgs.push_back("-vectorize-slp");
}
+
+void tools::handleInterchangeLoopsArgs(const ArgList &Args,
+ ArgStringList &CmdArgs) {
+ // FIXME: instead of relying on shouldEnableVectorizerAtOLevel, we may want
to
kkwli wrote:
Nit: instead → Instead
https://github.com/llvm/llvm-project/pull/140182
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
https://github.com/sebpop closed https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
https://github.com/tblah approved this pull request. LGTM, thanks https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
https://github.com/tarunprabhu approved this pull request. LGTM. Thanks! https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
https://github.com/sebpop updated
https://github.com/llvm/llvm-project/pull/140182
>From 46efee7d48a11794fc103cf67b21796d8e5f3408 Mon Sep 17 00:00:00 2001
From: Sebastian Pop
Date: Mon, 12 May 2025 21:56:03 +
Subject: [PATCH 1/3] [flang] add -floop-interchange to flang driver
This patch allows flang to recognize the flags -floop-interchange and
-fno-loop-interchange. -floop-interchange adds the loop interchange pass to the
pass pipeline.
---
clang/include/clang/Driver/Options.td | 4 ++--
clang/lib/Driver/ToolChains/Flang.cpp | 3 +++
flang/docs/ReleaseNotes.md | 2 ++
flang/include/flang/Frontend/CodeGenOptions.def | 1 +
flang/lib/Frontend/CompilerInvocation.cpp | 3 +++
flang/lib/Frontend/FrontendActions.cpp | 1 +
flang/test/Driver/loop-interchange.f90 | 7 +++
7 files changed, 19 insertions(+), 2 deletions(-)
create mode 100644 flang/test/Driver/loop-interchange.f90
diff --git a/clang/include/clang/Driver/Options.td
b/clang/include/clang/Driver/Options.td
index bd8df8f6a749a..c8c675bc17e7d 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4186,9 +4186,9 @@ def ftrap_function_EQ : Joined<["-"], "ftrap-function=">,
Group,
HelpText<"Issue call to specified function rather than a trap instruction">,
MarshallingInfoString>;
def floop_interchange : Flag<["-"], "floop-interchange">, Group,
- HelpText<"Enable the loop interchange pass">, Visibility<[ClangOption,
CC1Option]>;
+ HelpText<"Enable the loop interchange pass">, Visibility<[ClangOption,
CC1Option, FlangOption, FC1Option]>;
def fno_loop_interchange: Flag<["-"], "fno-loop-interchange">, Group,
- HelpText<"Disable the loop interchange pass">, Visibility<[ClangOption,
CC1Option]>;
+ HelpText<"Disable the loop interchange pass">, Visibility<[ClangOption,
CC1Option, FlangOption, FC1Option]>;
def funroll_loops : Flag<["-"], "funroll-loops">, Group,
HelpText<"Turn on loop unroller">, Visibility<[ClangOption, CC1Option,
FlangOption, FC1Option]>;
def fno_unroll_loops : Flag<["-"], "fno-unroll-loops">, Group,
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp
b/clang/lib/Driver/ToolChains/Flang.cpp
index b1ca747e68b89..c6c7a0b75a987 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -152,6 +152,9 @@ void Flang::addCodegenOptions(const ArgList &Args,
!stackArrays->getOption().matches(options::OPT_fno_stack_arrays))
CmdArgs.push_back("-fstack-arrays");
+ Args.AddLastArg(CmdArgs, options::OPT_floop_interchange,
+ options::OPT_fno_loop_interchange);
+
handleVectorizeLoopsArgs(Args, CmdArgs);
handleVectorizeSLPArgs(Args, CmdArgs);
diff --git a/flang/docs/ReleaseNotes.md b/flang/docs/ReleaseNotes.md
index b356f64553d7e..c76635d121d58 100644
--- a/flang/docs/ReleaseNotes.md
+++ b/flang/docs/ReleaseNotes.md
@@ -32,6 +32,8 @@ page](https://llvm.org/releases/).
## New Compiler Flags
+* -floop-interchange is now recognized by flang.
+
## Windows Support
## Fortran Language Changes in Flang
diff --git a/flang/include/flang/Frontend/CodeGenOptions.def
b/flang/include/flang/Frontend/CodeGenOptions.def
index d9dbd274e83e5..7ced60f512219 100644
--- a/flang/include/flang/Frontend/CodeGenOptions.def
+++ b/flang/include/flang/Frontend/CodeGenOptions.def
@@ -35,6 +35,7 @@ CODEGENOPT(PrepareForThinLTO , 1, 0) ///< Set when -flto=thin
is enabled on the
CODEGENOPT(StackArrays, 1, 0) ///< -fstack-arrays (enable the stack-arrays
pass)
CODEGENOPT(VectorizeLoop, 1, 0) ///< Enable loop vectorization.
CODEGENOPT(VectorizeSLP, 1, 0) ///< Enable SLP vectorization.
+CODEGENOPT(InterchangeLoops, 1, 0) ///< Enable loop interchange.
CODEGENOPT(LoopVersioning, 1, 0) ///< Enable loop versioning.
CODEGENOPT(UnrollLoops, 1, 0) ///< Enable loop unrolling
CODEGENOPT(AliasAnalysis, 1, 0) ///< Enable alias analysis pass
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp
b/flang/lib/Frontend/CompilerInvocation.cpp
index 238079a09ef3a..67fb0924def71 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -269,6 +269,9 @@ static void
parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts,
clang::driver::options::OPT_fno_stack_arrays, false))
opts.StackArrays = 1;
+ if (args.getLastArg(clang::driver::options::OPT_floop_interchange))
+opts.InterchangeLoops = 1;
+
if (args.getLastArg(clang::driver::options::OPT_vectorize_loops))
opts.VectorizeLoop = 1;
diff --git a/flang/lib/Frontend/FrontendActions.cpp
b/flang/lib/Frontend/FrontendActions.cpp
index e5a15c555fa5e..38dfaadf1dff9 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -922,6 +922,7 @@ void
CodeGenAction::runOptimizationPipeline(llvm::raw_pwrite_stream &os) {
if (ci.isTimingEnabled())
si.getTimePasses().setOutStream(ci.getTimingStreamLLVM
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
sjoerdmeijer wrote: > Thanks for this PR. Do you have any compilation time and performance data? This information is a bit spread out in the other tickets that I linked earlier, so to summarise that, compile times look really good and increases very minimal after the work that Madhur did. In https://github.com/llvm/llvm-project/pull/124911, I wrote: > The compile-time increase with a geomean increase of 0.19% looks good (after > committing https://github.com/llvm/llvm-project/pull/124247), I think: stage1-O3: Benchmark kimwitu+++0.10% sqlite3 +0.14% consumer-typeset +0.07% Bullet +0.06% tramp3d-v4 +0.21% mafft+0.39% ClamAVi +0.06% lencod +0.61% SPASS+0.17% 7zip +0.08% geomean +0.19% Regarding performance, as I also wrote in that ticket, loop-interchange has a lot of potential. It triggers a lot of times e.g. in the LLVM test-suite, see this https://github.com/llvm/llvm-project/pull/124911#issuecomment-2624704156. It is now triggering slightly less than what I wrote in that comment because we made interchange more pessimistic to fix correctness issues, but we think that's okay because we consider getting interchange and DependenceAnalysis running by default as a first enablement step. Once we have achieved this, we are going to focus on performance and lift some of the restrictions (while maintaining correctness of course). With this first patch, interchange won't trigger on SPEC for example, but we plan to do that as follow up. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
https://github.com/sebpop updated
https://github.com/llvm/llvm-project/pull/140182
>From b0a6935e8439bc5b4f742f55eb3bb090790a8f95 Mon Sep 17 00:00:00 2001
From: Sebastian Pop
Date: Wed, 7 May 2025 01:14:49 +
Subject: [PATCH 1/4] [flang] fix Werror=dangling-reference
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
when compiling with g++ 13.3.0 flang build fails with:
llvm-project/flang/lib/Semantics/expression.cpp:424:17: error: possibly
dangling reference to a temporary [-Werror=dangling-reference]
424 | const Symbol &coarraySymbol{ref.GetBase().GetLastSymbol()};
| ^
llvm-project/flang/lib/Semantics/expression.cpp:424:58: note: the temporary was
destroyed at the end of the full expression
‘Fortran::evaluate::CoarrayRef::GetBase()
const().Fortran::evaluate::NamedEntity::GetLastSymbol()’
424 | const Symbol &coarraySymbol{ref.GetBase().GetLastSymbol()};
| ~~~^~
Keep the base in a temporary variable to make sure it is not deleted.
---
flang/lib/Semantics/expression.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/flang/lib/Semantics/expression.cpp
b/flang/lib/Semantics/expression.cpp
index e139bda7e4950..35eb7b61429fb 100644
--- a/flang/lib/Semantics/expression.cpp
+++ b/flang/lib/Semantics/expression.cpp
@@ -421,7 +421,8 @@ static void CheckSubscripts(
static void CheckSubscripts(
semantics::SemanticsContext &context, CoarrayRef &ref) {
- const Symbol &coarraySymbol{ref.GetBase().GetLastSymbol()};
+ const auto &base = ref.GetBase();
+ const Symbol &coarraySymbol{base.GetLastSymbol()};
Shape lb, ub;
if (FoldSubscripts(context, coarraySymbol, ref.subscript(), lb, ub)) {
ValidateSubscripts(context, coarraySymbol, ref.subscript(), lb, ub);
>From c6d051a2b4239e1fe78e1d4483b500b129956867 Mon Sep 17 00:00:00 2001
From: Sebastian Pop
Date: Mon, 12 May 2025 21:56:03 +
Subject: [PATCH 2/4] [flang] add -floop-interchange to flang driver
This patch allows flang to recognize the flags -floop-interchange and
-fno-loop-interchange. -floop-interchange adds the loop interchange pass to the
pass pipeline.
---
clang/include/clang/Driver/Options.td | 4 ++--
clang/lib/Driver/ToolChains/Flang.cpp | 3 +++
flang/include/flang/Frontend/CodeGenOptions.def | 1 +
flang/lib/Frontend/CompilerInvocation.cpp | 3 +++
flang/lib/Frontend/FrontendActions.cpp | 1 +
flang/test/Driver/loop-interchange.f90 | 7 +++
6 files changed, 17 insertions(+), 2 deletions(-)
create mode 100644 flang/test/Driver/loop-interchange.f90
diff --git a/clang/include/clang/Driver/Options.td
b/clang/include/clang/Driver/Options.td
index 11677626dbf1f..287a00863bb35 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4141,9 +4141,9 @@ def ftrap_function_EQ : Joined<["-"], "ftrap-function=">,
Group,
HelpText<"Issue call to specified function rather than a trap instruction">,
MarshallingInfoString>;
def floop_interchange : Flag<["-"], "floop-interchange">, Group,
- HelpText<"Enable the loop interchange pass">, Visibility<[ClangOption,
CC1Option]>;
+ HelpText<"Enable the loop interchange pass">, Visibility<[ClangOption,
CC1Option, FlangOption, FC1Option]>;
def fno_loop_interchange: Flag<["-"], "fno-loop-interchange">, Group,
- HelpText<"Disable the loop interchange pass">, Visibility<[ClangOption,
CC1Option]>;
+ HelpText<"Disable the loop interchange pass">, Visibility<[ClangOption,
CC1Option, FlangOption, FC1Option]>;
def funroll_loops : Flag<["-"], "funroll-loops">, Group,
HelpText<"Turn on loop unroller">, Visibility<[ClangOption, CC1Option,
FlangOption, FC1Option]>;
def fno_unroll_loops : Flag<["-"], "fno-unroll-loops">, Group,
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp
b/clang/lib/Driver/ToolChains/Flang.cpp
index b1ca747e68b89..c6c7a0b75a987 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -152,6 +152,9 @@ void Flang::addCodegenOptions(const ArgList &Args,
!stackArrays->getOption().matches(options::OPT_fno_stack_arrays))
CmdArgs.push_back("-fstack-arrays");
+ Args.AddLastArg(CmdArgs, options::OPT_floop_interchange,
+ options::OPT_fno_loop_interchange);
+
handleVectorizeLoopsArgs(Args, CmdArgs);
handleVectorizeSLPArgs(Args, CmdArgs);
diff --git a/flang/include/flang/Frontend/CodeGenOptions.def
b/flang/include/flang/Frontend/CodeGenOptions.def
index d9dbd274e83e5..7ced60f512219 100644
--- a/flang/include/flang/Frontend/CodeGenOptions.def
+++ b/flang/include/flang/Frontend/CodeGenOptions.def
@@ -35,6 +35,7 @@ CODEGENOPT(PrepareForThinLTO , 1, 0) ///< Set when -flto=thin
is enabled on the
CODEGENOPT(StackArrays, 1, 0) ///< -fstack-arrays (enable the stack-arrays
pass)
CODEGENOPT(VectorizeLoop, 1, 0) ///< Enable loop vectorization.
CODEGENOP
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
https://github.com/tarunprabhu edited https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
https://github.com/tarunprabhu commented: Could you add a test that ensures that the loop-interchange pass is added to the pipeline. Perhaps something like [flang/test/Driver/slp-vectorize.f90](https://github.com/llvm/llvm-project/blob/04fde85057cb4da2e560da629df7a52702eac489/flang/test/Driver/slp-vectorize.f90#L9) https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
@@ -421,7 +421,8 @@ static void CheckSubscripts(
static void CheckSubscripts(
semantics::SemanticsContext &context, CoarrayRef &ref) {
- const Symbol &coarraySymbol{ref.GetBase().GetLastSymbol()};
+ const auto &base = ref.GetBase();
+ const Symbol &coarraySymbol{base.GetLastSymbol()};
tarunprabhu wrote:
I requested a review for #138793. It's probably best to proceed with this after
that has been merged.
https://github.com/llvm/llvm-project/pull/140182
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
sebpop wrote: > Do you have any compilation time and performance data? @madhur13490 did several changes to loop interchange to optimize the overall compilation time with the pass. I believe Madhur has only looked at c/c++ benchmarks and not at how loop interchange would impact flang. I think that if compilation time is good for c/c++, it should also be good for fortran. On the perf side, I was looking if we can already catch swim from cpu2000, and that fails with not enough data to infer number of iterations. I will be working on adding assume (N < 1335) based on analyzing array decls and infer loop bounds. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
@@ -421,7 +421,8 @@ static void CheckSubscripts(
static void CheckSubscripts(
semantics::SemanticsContext &context, CoarrayRef &ref) {
- const Symbol &coarraySymbol{ref.GetBase().GetLastSymbol()};
+ const auto &base = ref.GetBase();
+ const Symbol &coarraySymbol{base.GetLastSymbol()};
sebpop wrote:
This has been submitted separately
https://github.com/llvm/llvm-project/pull/138793
Without this change I cannot build flang on arm64-linux ubuntu 24.04 machine.
https://github.com/llvm/llvm-project/pull/140182
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
https://github.com/kiranchandramohan edited https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
@@ -421,7 +421,8 @@ static void CheckSubscripts(
static void CheckSubscripts(
semantics::SemanticsContext &context, CoarrayRef &ref) {
- const Symbol &coarraySymbol{ref.GetBase().GetLastSymbol()};
+ const auto &base = ref.GetBase();
+ const Symbol &coarraySymbol{base.GetLastSymbol()};
kiranchandramohan wrote:
Nit: Is this an unrelated change?
https://github.com/llvm/llvm-project/pull/140182
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
https://github.com/kiranchandramohan commented: Thanks for this PR. Do you have any compilation time and performance data? https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] add -floop-interchange and enable it with opt levels (PR #140182)
sjoerdmeijer wrote: For more context, this is part of our loop-interchange enablement story, see our RFC here: https://discourse.llvm.org/t/enabling-loop-interchange/82589. We have fixed all the compile-time issues and loop-interchange issues that we are aware of, and would like to enable this in the C/C++ flow, see here: https://github.com/llvm/llvm-project/pull/124911. As part of this work, we also promised to fix DependenceAnalysis. The last DA correctness corner-case that is being worked on is: https://github.com/llvm/llvm-project/pull/123436. This is a corner-case for C/C++ related to type punning, different offset sizes that won't be a problem in Fortran. Therefore, we think that enabling interchange and dependence analysis for Fortran makes sense. https://github.com/llvm/llvm-project/pull/140182 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
