This is an archive of the discontinued LLVM Phabricator instance.

[flang][directives] Use TableGen to generate clause unparsing
ClosedPublic

Authored by clementval on Aug 12 2020, 12:30 PM.

Details

Summary

Use the TableGen directive back-end to generate code for the clauses unparsing.

Diff Detail

Event Timeline

clementval created this revision.Aug 12 2020, 12:30 PM
Herald added a project: Restricted Project. · View Herald Transcript
clementval requested review of this revision.Aug 12 2020, 12:30 PM
clementval added a project: Restricted Project.Aug 12 2020, 12:31 PM

Do you plan to update the tests?

flang/include/flang/Parser/parse-tree.h
3473

Would it make sense to move dist_schedule out of OmpClause and have it as a separate struct?

Do you plan to update the tests?

Yeah I forgot that. Will update the patch today with updated test.

flang/include/flang/Parser/parse-tree.h
3473

Yeah I thought about doing that. Might make it clearer that it's a specific case and doesn't fit in the generic ones.

Extract DistSchedule from OmpClause + add tests

LGTM. I have mentioned a few nits. You also have some clang-tidy warnings to fix.

BTW, the tests do not seem to be using the new fields introduced in this patch. Would it make sense to use them in the test?

bit valueIsList = 0;
string defaultValue = "";
llvm/utils/TableGen/DirectiveEmitter.cpp
43

Nit: Should the name be isValueList to remain consistent with other boolean functions?

475–484

Nit: Was just checking whether this code can be improved.
At least printing of ");\n" can be common.

529

Nit: as -> is

This revision is now accepted and ready to land.Aug 13 2020, 3:33 PM
clementval marked 4 inline comments as done.

Address review comment

@kiranchandramohan Thanks for the review. I have added test and corrected other stuffs. One clang-today warning will remain for the function name. It is not consistent with the rest of TableGen so I ignored it from the beginning.

sscalpone accepted this revision.Aug 15 2020, 9:02 AM

Looks good!

This revision was landed with ongoing or failed builds.Aug 17 2020, 11:22 AM
This revision was automatically updated to reflect the committed changes.