Page MenuHomePhabricator

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.Tue, May 28, 7:37 AM

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

AndreyChurbanov requested changes to this revision.Tue, May 28, 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.Tue, May 28, 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.Tue, May 28, 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.Tue, May 28, 1:33 PM

Added a test.

thanks for test, LGTM

This revision is now accepted and ready to land.Wed, May 29, 1:09 AM
Closed by commit rOMP362125: Fix OMP_TARGET_OFFLOAD parsing (authored by hbae, committed by ). · Explain WhyThu, May 30, 11:32 AM
This revision was automatically updated to reflect the committed changes.