This is an archive of the discontinued LLVM Phabricator instance.

[flang][openacc] Handle optional end directive in combined construct
ClosedPublic

Authored by clementval on Jul 23 2020, 6:24 PM.

Details

Summary

OpenACC combined construct can have an optional end directive. This patch handle this
case in the parsing/unparsing with a canonicalization step. Unlike OmpEndLoopDirective,
this doesn't need a special treatment in the pre-fir tree as there is no clause attached to
a AccEndCombinedDirective.

Diff Detail

Event Timeline

clementval created this revision.Jul 23 2020, 6:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 23 2020, 6:24 PM

As discussed in D84195 with @ichoyjx.

Remove pre-fir tree AccEndCombinedDirective since it is not necessary

clementval edited the summary of this revision. (Show Details)Jul 24 2020, 8:35 AM
DavidTruby resigned from this revision.Jul 31 2020, 6:26 AM
flang/lib/Semantics/canonicalize-acc.cpp
89

#just-asking: Is it possible to combine this with RewriteOpenACCLoopConstruct and the Canonicalisation for OpenMP loop?

clementval added inline comments.Jul 31 2020, 1:22 PM
flang/lib/Semantics/canonicalize-acc.cpp
89

I guess if we start at a higher level of construct like OpenACCConstruct we can merge the loop and combined construct rewrite. To reduce the code duplication between canonicalization we can extract the rewriting code in a templated function but I'm assuming we still need the rewrite functions to call the templated function then. I'll try to have a look at it this week-end if I find time. Always good if we can reduce the amount of code that is duplicated.

clementval added a project: Restricted Project.
klausler added inline comments.Aug 6 2020, 5:45 PM
flang/lib/Parser/program-parsers.cpp
80–83

Can an OpenACC end directive really be the first statement in the executable part?

clementval added inline comments.Aug 6 2020, 6:56 PM
flang/lib/Parser/program-parsers.cpp
80–83

Only if the user makes a big mistake. I guess same with ompEndLoopDirective. I'm fine to remove it. It was to do as omp loop was done.

clementval added inline comments.Aug 12 2020, 12:20 PM
flang/lib/Parser/program-parsers.cpp
80–83

@klausler So should we remove both ompEndLoopDirective and accEndCombinedDirective ?

klausler added inline comments.Aug 12 2020, 3:50 PM
flang/lib/Parser/program-parsers.cpp
80–83

Please do so; thanks.

80–83

Yes, please.

clementval marked 3 inline comments as done.

Address review comment

@klausler The patch is updated.

flang/lib/Parser/program-parsers.cpp
80–83

Removed in the updated patch

klausler accepted this revision.Aug 13 2020, 8:54 AM

Thanks!

This revision is now accepted and ready to land.Aug 13 2020, 8:54 AM