This is an archive of the discontinued LLVM Phabricator instance.

[OPENMP]Initial support for at clause
ClosedPublic

Authored by jyu2 on Nov 11 2022, 9:14 AM.

Details

Summary

Initial support for at clause.

Error directive is allowed in both declared and executable context.
The function ActOnOpenMPAtClause is called in both places during the parsers.

Adding a param “bool InExContext” to identify context which is used to emit error massage.

Diff Detail

Event Timeline

jyu2 created this revision.Nov 11 2022, 9:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 11 2022, 9:14 AM
jyu2 requested review of this revision.Nov 11 2022, 9:14 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 11 2022, 9:14 AM
ABataev added inline comments.Nov 11 2022, 9:35 AM
clang/include/clang/AST/OpenMPClause.h
1613–1615

Make it private

clang/include/clang/Parse/Parser.h
3296–3304

Could you make it in a separate NFC patch?

clang/lib/Sema/SemaOpenMP.cpp
11034–11037

find_first_of?

jyu2 updated this revision to Diff 474826.Nov 11 2022, 12:04 PM

Thanks Alexey, for the review. This is addressing his comments.

Herald added a project: Restricted Project. · View Herald TranscriptNov 11 2022, 12:04 PM
jyu2 added inline comments.Nov 11 2022, 12:05 PM
clang/include/clang/AST/OpenMPClause.h
1613–1615

Thanks. Yes. Changed

clang/include/clang/Parse/Parser.h
3296–3304

Changed.

clang/lib/Sema/SemaOpenMP.cpp
11034–11037

Thanks. Changed.

jyu2 added inline comments.Nov 11 2022, 2:31 PM
clang/lib/Basic/OpenMPKinds.cpp
102

I run git-clang-format I don't see any change. I don't know where has problem here.

clang>tools/clang-format/git-clang-format HEAD~
clang-format did not modify any files
clang>

jyu2 updated this revision to Diff 474874.Nov 11 2022, 3:23 PM

Fix format problem.

jyu2 updated this revision to Diff 475013.Nov 13 2022, 11:57 AM

try fix format again

ABataev added inline comments.Nov 14 2022, 4:27 AM
clang/include/clang/Parse/Parser.h
3302

SmallVectorImpl<OMPClause *> &

clang/lib/Parse/ParseOpenMP.cpp
1656–1657

Better to diagnose it in Sema

jyu2 updated this revision to Diff 475177.Nov 14 2022, 9:28 AM

Thanks Alexey!

clang/include/clang/Parse/Parser.h
3302

Thanks!! Changed.

clang/lib/Parse/ParseOpenMP.cpp
1656–1657

Move to Sema!! Changed.

ABataev added inline comments.Nov 14 2022, 9:39 AM
clang/include/clang/Sema/Sema.h
11295

Why do you need this flag?

clang/lib/Sema/SemaOpenMP.cpp
11035–11037

Why do you need a loop here?

jyu2 updated this revision to Diff 475225.Nov 14 2022, 11:28 AM

Thanks Alexey! Address Alexey comments.

clang/include/clang/Sema/Sema.h
11295

The error pragma is not allowed in declared context when "at" clause with execution modifier.
It is allowed with compilation modifier.

clang/lib/Sema/SemaOpenMP.cpp
11035–11037

Only one "at" clause is allowed. So I think Loop does not matters. Changed.

Drive by: Please add commit messages to non trivial changes to explaining what's going on.

jyu2 edited the summary of this revision. (Show Details)Nov 14 2022, 12:23 PM
jyu2 added a comment.Nov 14 2022, 12:26 PM

Drive by: Please add commit messages to non trivial changes to explaining what's going on.

Thanks. Added.

ABataev added inline comments.Nov 15 2022, 3:48 AM
clang/lib/Parse/ParseOpenMP.cpp
2168

InExContext

clang/lib/Sema/SemaOpenMP.cpp
11036

AtClauses.front()

jyu2 updated this revision to Diff 475490.Nov 15 2022, 8:09 AM

Thanks Alexey for the review. This is address his comments.

clang/lib/Parse/ParseOpenMP.cpp
2168

Sorry. Changed

This revision is now accepted and ready to land.Nov 15 2022, 8:22 AM
This revision was automatically updated to reflect the committed changes.