This is an archive of the discontinued LLVM Phabricator instance.

[clang][OpenMP] OMPFlushClause is synthetic, no such clause exists
ClosedPublic

Authored by lebedev.ri on Jan 26 2019, 1:51 AM.

Details

Summary

As discussed in https://reviews.llvm.org/D57112#inline-506781,
'flush' clause does not exist in the OpenMP spec, it can not be
specified, and OMPFlushClause class is just a helper class.

Therefore OPENMP_CLAUSE() in clang/Basic/OpenMPKinds.def
should not contain 'flush' "clause".

I have simply removed the OPENMP_CLAUSE(flush, OMPFlushClause)
from clang/Basic/OpenMPKinds.def, grepped for OPENMP_CLAUSE
and added OPENMP_CLAUSE(flush, OMPFlushClause) back to the every
place where OPENMP_CLAUSE is defined and clang/Basic/OpenMPKinds.def
is then included.

So as-is, this patch is a NFC. Possibly, some of these
OPENMP_CLAUSE(flush, OMPFlushClause) should be dropped,
i don't really know.

Test plan: ninja check-clang

Diff Detail

Repository
rL LLVM

Event Timeline

lebedev.ri created this revision.Jan 26 2019, 1:51 AM
ABataev accepted this revision.Jan 28 2019, 6:19 AM

LG with a nit

lib/Basic/OpenMPKinds.cpp
56 ↗(On Diff #183689)

Just .Case("flush", OMPC_flush), similar to uniform

This revision is now accepted and ready to land.Jan 28 2019, 6:19 AM

LG with a nit

Thank you for the review!

lebedev.ri added inline comments.Jan 28 2019, 8:03 AM
lib/Basic/OpenMPKinds.cpp
56 ↗(On Diff #183689)

Hm, we won't ever trigger that case, since we have if (Str == "flush") return OMPC_unknown; before this.

ABataev added inline comments.Jan 28 2019, 8:07 AM
lib/Basic/OpenMPKinds.cpp
56 ↗(On Diff #183689)

Then just remove this new line completely.

lebedev.ri marked an inline comment as done.Jan 28 2019, 9:04 AM
lebedev.ri added inline comments.
lib/Basic/OpenMPKinds.cpp
56 ↗(On Diff #183689)

Yep, great idea, did that.

This revision was automatically updated to reflect the committed changes.