Page MenuHomePhabricator

[flang][NFC] Extract name resolution for OpenACC & OpenMP into new file
ClosedPublic

Authored by tskeith on Jul 28 2020, 10:14 AM.

Details

Summary

Move ResolveAccParts and ResolveOmpParts from resolve-names.cpp to
resolve-directives.{h,cpp}. Move the implementation in the classes
DirectiveAttributeVisitor, AccAttributeVisitor, and
OmpAttributeVisitor to resolve-directives.cpp as well.

To allow this to happen, move EvaluateIntExpr and introduce
EvaluateInt64 to resolve-names-utils.h. The latter is also useful
elsewhere in resolve-names.cpp for converting an Expr to std::int64_t.

The other problem was that ResolveDesignator was called from the code
that was moved. At the moment it doesn't seem to be doing anything so I
removed the calls (and no tests failed). If it proves to be needed, we
can either resolve those designators in resolve-names.cpp or pass the
ResolveDesignator function in to the code that needs to call it.

Diff Detail

Event Timeline

tskeith created this revision.Jul 28 2020, 10:14 AM
tskeith requested review of this revision.Jul 28 2020, 10:14 AM

On my laptop with this change I see resolve-names.cpp taking about 15% less time and memory to compile.

clementval accepted this revision.Jul 28 2020, 11:20 AM

Thanks for working on this. LGTM besides the two small clang-tidy warnings that were in the original code anyway. It is maybe good if @ichoyjx gives an inputs.

flang/lib/Semantics/resolve-directives.cpp
310

Add const here as suggested?

770

Add const here as suggested?

This revision is now accepted and ready to land.Jul 28 2020, 11:20 AM
tskeith added inline comments.Jul 28 2020, 12:00 PM
flang/lib/Semantics/resolve-directives.cpp
310

OK

770

OK

ichoyjx accepted this revision.Jul 28 2020, 4:25 PM

I called ResolveDesignator just in case the designator is not resolved before doing further checks. I agree with Tim that we can assume it is until we find a case where it’s not.

tskeith updated this revision to Diff 281413.Jul 28 2020, 4:36 PM

Fix clang-tidy warnings. Remove unused ResolveVariable.

This revision was landed with ongoing or failed builds.Jul 28 2020, 4:38 PM
This revision was automatically updated to reflect the committed changes.