Compiler Engineer at AMD
User Details
- User Since
- Dec 24 2019, 1:39 AM (205 w, 12 h)
Sep 5 2023
Moved to https://github.com/llvm/llvm-project/pull/65380 because phabricator has been unresponsive lately (I have to hit reply multiple times before it sends one).
I realized that we cannot really do the first approach, because unroll could be present without nested loops. The nesting of openmp constructs could help with this. It might require complex indexing though. @kiranchandramohan do we have the details on loop-fission or the loop-modifiers for apply (like intratile)? I checked the technical report document here but it doesn't mention too much detail about these modifiers.
There are two possible approaches for the unrolled operation here -
%unrolled1 = omp.unroll loops(%tiled) at(1) %unrolled2 = omp.unroll loops(%tiled#1)
Aug 29 2023
I like @jsjodin's idea about nesting these. It avoids generating problematic IR like this
%cli1, %cli2, %cli3 = omp.canonical_loop ... { %cli2, %cli3 = omp.canonical_loop ... { %cli3 = omp.canonical_loop ... { } omp.yield(%cli3) } omp.yield(%cli2, %cli3) } %mergedcli = omp.collapse(%cli1, %cli3) // Problem here - collapsing innermost and outermost loops
Aug 28 2023
Fix build by adding enums and attributes in CMakeLists.txt
Address comments and nits
After discussing with Michael, I will be trying to implement this operation. @domada I have added an example of intended use in the operation with teams distribute construct. Hope it helps.
Please make sure that there are testcases for all the checks that have checkExclusiveLists.
Thank you for working on this @skatrak. I will abandon this patch now.
Aug 25 2023
Hi @Meinersbur. Thank you for this patch. This looks like it would make the IR cleaner. It isn't clear to me how the loop info result of this operation is going to be used in the IR. Can you please elaborate on that? Also, an example of how it would look under omp.parallel would be helpful to understand.
Jul 13 2023
Abandoning this.
Apr 17 2023
Apr 16 2023
Apr 15 2023
Jan 17 2023
Please add a test case which would fail with the old implementation but passes with the new one.
Oct 13 2022
Sep 22 2022
Rebase with main. Thanks for the review @raghavendhra.
Ping for review!
Sep 15 2022
Thanks for the patch @SouraVX. LGTM.
Sep 13 2022
Ping for review.
Sep 4 2022
Rebase with main. Adressed comment.
Aug 31 2022
Aug 30 2022
Addressed review comment.
Addressed review comment.
Rebase with main
Aug 25 2022
Apologies for the delay in review. Please let me know what you think. I will join the next OpenMP call and we can maybe discuss this.
Aug 15 2022
Aug 14 2022
Minor comments. Thank you for working on this!
Aug 13 2022
Ping for review.
Aug 9 2022
Jul 29 2022
LGTM, minor comments. Please let me know what you think!
Jul 28 2022
Removed NoSideEffect.
Rebase with main
Jul 27 2022
LGTM.
Addressed comments.
Jul 26 2022
I have reverted this change for now, we can discuss and add TODOs for the unhandled cases and then merge it.
Jul 25 2022
Jul 22 2022
Just a thought - for the long run, would it make sense to have OpenMPConverter class inheriting from FirConverter and overriding some functions- like genFIR for OpenMP constructs and genFIR for assignment statement? IMO this would make the file more structured, while separating OpenMP and Fortran code. Right now, OpenMP.cpp is just a bunch of functions.
Jul 21 2022
Ping for review. (This is not urgent for the drop on July 26th, and patches related to those are high priority. This is just a gentle reminder, if anyone is willing to look into this patch. :) )
Jul 20 2022
Rebase with main
Jul 19 2022
Nevermind, this has been fixed in https://github.com/llvm/llvm-project/commit/2c488a6b35c6d03e34fdd4e7ec87ed6bdc3d3010
Jul 18 2022
Moved checking to isSupportedByOpenMPIRBuilder.
Rebase with main
Jul 14 2022
Any pointer to this presentation?
This was discussed today in the OpenMP work for llvm-flang call.
Jul 12 2022
Jul 11 2022
Jul 10 2022
Ping for review.
Jul 6 2022
Jul 5 2022
LGTM. Thank you!
Jul 4 2022
The generated code in testcases looks good. Please wait for @peixin's approval for the code part, but functionality-wise this LGTM.