Move reduction lowering from DistributeOp to TeamsOp and use teams reduction clauses to generate info. #159
Conversation
jsjodin
requested review from
skatrak,
agozillon,
DominikAdamski,
dpalermo and
TIFitis
| @@ -0,0 +1,15 @@ | |||
| ! RUN: %flang_fc1 -emit-fir -fopenmp -o - %s | FileCheck %s | |||
There was a problem hiding this comment.
Nit: Maybe we can name this test "reduction-target-spmd.f90" and keep another simpler "reduction-teams.f90" with the same example that was removed from the "Todo" subdirectory.
| RI.ReductionGen(Builder.saveIP(), RHSValue, LHSValue, Reduced); | ||
| Builder.CreateStore(Reduced, LHS, false); | ||
| } | ||
| Value *LHSValue = Builder.CreateLoad(RI.ElementType, LHS, "final.lhs"); |
There was a problem hiding this comment.
Nit: Remove HasDistribute function argument, since it's no longer necessary.
| bool isNowait = false, bool isTeamsReduction = false, | ||
| bool hasDistribute = false) { |
There was a problem hiding this comment.
| bool isNowait = false, bool isTeamsReduction = false, | |
| bool hasDistribute = false) { | |
| bool isNowait = false, bool isTeamsReduction = false) { |
| // FIXME: This treats 'DO SIMD' as if it was a 'DO' construct. Reductions | ||
| // on other constructs apart from 'DO' aren't considered either. |
There was a problem hiding this comment.
Nit: We can remove this comment now, since we're no longer looking for the wrappers of an omp.loop_nest.
| } | ||
|
|
||
| template <typename OpTy> | ||
| static uint64_t getReductionDataSize(OpTy &op) { |
There was a problem hiding this comment.
Don't we currently need this for other reductions? If not, do we expect to use it elsewhere or is this actually only needed to be calculated for teams reductions within a target region?
There was a problem hiding this comment.
I think it is only needed for the team reduction, but I figured if we ever want to know the data size for parallel/workshare it doesn't hurt to parameterize the code.
| return convertTargetDeviceOp(op, builder, moduleTranslation, | ||
| dispatchList); | ||
| return convertTargetOpsInNest(op, builder, moduleTranslation); | ||
| if (isTargetDeviceOp(op)) { |
There was a problem hiding this comment.
Nit: I think the previous formatting is the preferred one (no unnecessary braces and no 'else' after 'return').
…clause for the teams directive to create the reduction information.
…th the new teams reduction implementation.
jsjodin
force-pushed
the
jleyonberg/teams-reduction
branch
from
3861586
to
2287513
Compare
This patch moves the reduction lowering from the DistributeOp to the TeamsOp by enabling the reduction clauses on the TeamsOp and generate the reductions from those clauses. This makes the composite code generation obsolete since the reduction information can be recomputed. This change should also allow having separate teams and wsloop reductions in the same block.
Co-author: @skatrak