Page MenuHomePhabricator

[openacc][openmp] Allow duplicate between required and allowed once/exclusive
ClosedPublic

Authored by clementval on Wed, Nov 4, 8:55 AM.

Details

Summary

Validity check introduce in D90241 are a bit too restrict and this patch propose to losen
them a bit. The duplicate clauses is now check only between the three allowed lists and between the
requiredClauses and allowedClauses lists. This allows to enable some check where a clause can be
required but also appear only once on the directive. We found these kind of restriction useful
on the set directive in OpenACC for example.

Diff Detail

Event Timeline

clementval created this revision.Wed, Nov 4, 8:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Nov 4, 8:55 AM
clementval requested review of this revision.Wed, Nov 4, 8:55 AM
kiranchandramohan accepted this revision.Thu, Nov 5, 7:17 AM

LGTM.

llvm/utils/TableGen/DirectiveEmitter.cpp
161–168

Nit: Does the return value match the name of this function?

Shouldn't it be like the following?
if records are valid then it is all good (return true).
If records are not valid then it is bad (return false).

This revision is now accepted and ready to land.Thu, Nov 5, 7:17 AM
clementval added inline comments.Thu, Nov 5, 7:21 AM
llvm/utils/TableGen/DirectiveEmitter.cpp
161–168

Right it is misleading. I'll update that to be consistent between the name and the return value.

sameeranjoshi added inline comments.Thu, Nov 5, 10:18 AM
llvm/utils/TableGen/DirectiveEmitter.cpp
134

nit: - Is clang-tidy suggesting something? if not ignore me on this.

clementval added inline comments.Thu, Nov 5, 11:42 AM
llvm/utils/TableGen/DirectiveEmitter.cpp
134

Good catch. clang-tidy is not followed always in TableGen backends but this one should be. Thanks!

clementval marked 2 inline comments as done.

Rebase + address review comments

This revision was landed with ongoing or failed builds.Thu, Nov 5, 1:21 PM
This revision was automatically updated to reflect the committed changes.