This is an archive of the discontinued LLVM Phabricator instance.

Fix OMP_TARGET_OFFLOAD parsing
ClosedPublic

Authored by hbae on May 24 2019, 3:34 PM.

Details

Summary

Current parsing allows trailing string after the permitted value MANDATORY|DISABLED|DEFAULT -- e.g., mandatorynot is also recognized as MANDATORY.
Such cases should be recognized as incorrect/unknown value.

Diff Detail

Repository
rOMP OpenMP

Event Timeline

hbae created this revision.May 24 2019, 3:34 PM

looks like a good change
also consistent with some recent OpenMP language examples committee discussions on this topic.

This revision is now accepted and ready to land.May 28 2019, 7:37 AM

one follow on question, is there value in adding a testcase for this patch ?

AndreyChurbanov requested changes to this revision.May 28 2019, 9:17 AM

Probably test case worth adding. Or even two, - one negative with something like "mandatorynot" value, another positive (we do have internal getter routine for the target-offload-var ICV).

This revision now requires changes to proceed.May 28 2019, 9:17 AM

By negative test I meant the wrong value is ignored and the default used instead. I don't think we need to check library warnings.

hbae added a comment.May 28 2019, 9:38 AM

Yes, I agree that it is good to have tests for this -- I will be updating the patch with tests.

hbae updated this revision to Diff 201749.May 28 2019, 1:33 PM

Added a test.

thanks for test, LGTM

This revision is now accepted and ready to land.May 29 2019, 1:09 AM
This revision was automatically updated to reflect the committed changes.