This is an archive of the discontinued LLVM Phabricator instance.

[flang][openacc] Enforce delcare directive restriction
ClosedPublic

Authored by clementval on Dec 6 2020, 5:37 PM.

Details

Summary

Add semantic check for most of the restrictions for the declare directive.

Diff Detail

Event Timeline

clementval requested review of this revision.Dec 6 2020, 5:37 PM
clementval created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptDec 6 2020, 5:37 PM

Remove useless code

Thanks for the patch.
A few comments below.

I came to know quite a few new things from Flang code base.

  • How to check scoping.
  • Given an ObjectList or similar representation how to extract symbol and use interfaces from tools.h.
flang/lib/Semantics/resolve-directives.cpp
116–117

Trying to understand- not a blocking review comment.
The Walk should be handled in some tree-visitor files and not here, is my understanding correct?

484

Possibly a bug.
Is this Copyout ?

504

I see in my OACC 3.1 copy create clause as one of them in the list.
Is this missing to add over here with a test below ?

513

Do you need somewhere the index into standard for this?
2414 from 3.1 spec.

514

Are checks from line 2410-2411 and 2412-2413 from OACC 3.1 missing or are handled somewhere else in previous patches ?

flang/test/Semantics/acc-declare-validity.f90
54–56

Any plans to add these ?
Until then can you add a TODO or remove them?

sameeranjoshi added a project: Restricted Project.Dec 16 2020, 1:56 AM
clementval marked 5 inline comments as done.

Rebase + address review comment

Thanks for the review. Did not see it before for some reason. I made some update to the patch.

flang/lib/Semantics/resolve-directives.cpp
116–117

Yeah the Walk is already handled by the Visitor class inherited by the DirectiveAttributeVisitor

484

Good catch!

504

Added it.

514

Those are not yet implemented and will be done in a separate patch.

kiranktp accepted this revision.Jan 7 2021, 3:08 AM

LGTM

This revision is now accepted and ready to land.Jan 7 2021, 3:08 AM
This revision was automatically updated to reflect the committed changes.