This patch implements collapsing of the loops (in particular, in presense of clause 'collapse'). It calculates number of iterations N and expressions nesessary to calculate the nested loops counters values based on new iteration variable (that goes from 0 to N-1) in Sema. It also adds Codegen for 'omp simd', which uses (and tests) this feature.
Details
- Reviewers
kkwli0 rjmccall • cbergstrom schandra ABataev • fraggamuffin • ejstotzer hfinkel rsmith doug.gregor - Commits
- rGa5f070aec038: [OPENMP] Loop collapsing and codegen for 'omp simd' directive.
rC218743: [OPENMP] Loop collapsing and codegen for 'omp simd' directive.
rL218743: [OPENMP] Loop collapsing and codegen for 'omp simd' directive.
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/CodeGen/CGStmtOpenMP.cpp | ||
---|---|---|
67 ↗ | (On Diff #14071) | Is there something missing for you to support this? Looks like everything is in place below. |
lib/Sema/SemaOpenMP.cpp | ||
2534 ↗ | (On Diff #14071) | The OpenMP specification says: The integer type (or kind, for Fortran) used to compute the iteration count for the collapsed loop is implementation defined. And I'm afraid that, especially if the iteration variables are originally 32-bits, with the current scheme we'll overflow when calculating the total number of iterations. Can we always use a larger integer type? |
Hal,
I updated the code to make the number of iterations 64-bit if we cannot prove that 32 bits would be enough and added a test with such example.
I'm not sure if it's worth to do widening to 128 bits in similar cases with several nested loops with 64-bit counters - in particular, in case of worksharing loop directives the library only supports 32-bit and 64-bit counters, so we'll need truncate it to 64 bits for such directives anyway.
Regarding 'lastprivate' codegen at: lib/CodeGen/CGStmtOpenMP.cpp:67 -- this will need helper expressions in OMPLastprivateClause (e.g., Copy Constructor to copy from private var to the non-private), this would be better to do in a separate patch.
Thanks,
Alexander
There will be a separate patch for lastprivate variables - the codegen for lastprivates is very similar to codegen for privates and firstprivates. I'll prepare it as soon as codegen for privates and firstprivates is approved and committed.
LGTM, thanks!
Regarding 'lastprivate' codegen at: lib/CodeGen/CGStmtOpenMP.cpp:67 -- this will need helper expressions in OMPLastprivateClause (e.g., Copy Constructor to copy from private var to the non-private), this would be better to do in a separate patch.
Okay, please make this clear in the comment (and please add an assert, to be removed later, if the code will be functionally-incorrect without this logic).
lib/Sema/SemaOpenMP.cpp | ||
---|---|---|
2597 ↗ | (On Diff #14210) | Add "the" before 32-bit |