Page MenuHomePhabricator

OpenMP 5.1 omp_display_env function implementation.
ClosedPublic

Authored by AndreyChurbanov on Feb 21 2020, 4:50 AM.

Details

Summary

Patch by Michael Klemm.

It implements omp_display_env() API, which is run time counterpart of the OMP_DISPLAY_ENV environment variable.
This is part of coming OpenMP 5.1 specification to be released later this year.

Diff Detail

Event Timeline

Added full context.
Also added (missed) "value" attribute to Fortran interfaces.

kkwli0 added a subscriber: kkwli0.Feb 21 2020, 7:07 AM
kkwli0 added inline comments.
openmp/runtime/src/include/omp_lib.f.var
493

The subroutine is declared as bind(C) and the verbose argument is passed by reference. Does it match the C prototype? Should it have the value attribute as well?

Addressed Kelvin's comment.

AndreyChurbanov marked an inline comment as done.Feb 21 2020, 7:21 AM

We can have filechek tests, right? We should verify a bit more than the existence of a function that will return.

openmp/runtime/src/kmp_settings.cpp
5761

Do we need to keep this code?

We can have filechek tests, right? We should verify a bit more than the existence of a function that will return.

I am not sure how to pipe error output (stderr) to filecheck. I saw some examples of the "2>&1 |", this works in bash, but does not work in c shell. Maybe "|2" can be used?

Could somebody please advise how to better pipe stderr to filecheck? Is the "2>&1 |" appropriate? I personally work in bash usually, but some systems may have other shell by default.

hbae added a comment.Mar 2 2020, 11:47 AM

We can have filechek tests, right? We should verify a bit more than the existence of a function that will return.

I am not sure how to pipe error output (stderr) to filecheck. I saw some examples of the "2>&1 |", this works in bash, but does not work in c shell. Maybe "|2" can be used?

Could somebody please advise how to better pipe stderr to filecheck? Is the "2>&1 |" appropriate? I personally work in bash usually, but some systems may have other shell by default.

Can we do it in two steps?

  1. Redirect 2 to file A
  2. Then, feed A to FileCheck

We can have filechek tests, right? We should verify a bit more than the existence of a function that will return.

I am not sure how to pipe error output (stderr) to filecheck. I saw some examples of the "2>&1 |", this works in bash, but does not work in c shell. Maybe "|2" can be used?

Could somebody please advise how to better pipe stderr to filecheck? Is the "2>&1 |" appropriate? I personally work in bash usually, but some systems may have other shell by default.

Lit parses the 2>&1 directly, redirection is set (probably for subprocess python module), and the text is not sent to an underlying shell. So lit acts as a limited, platform-independent Bourne shell. So It is safe to use 2>&1.

From https://llvm.org/docs/TestingGuide.html:

RUN lines are specified in the comments of the test program using the keyword `RUN` followed by a colon, and lastly the command (pipeline)
to execute. Together, these lines form the "script" that :program:lit executes to run the test case. The syntax of the RUN lines is similar to a
shell's syntax for pipelines including I/O redirection and variable substitution. However, even though these lines may *look* like a shell
script, they are not. RUN lines are interpreted by :program:lit. Consequently, the syntax differs from shell in a few ways. You can specify
as many RUN lines as needed.

Addressed comments: removed "#if 0" code block; added basic checks for the test.

AndreyChurbanov marked an inline comment as done.Mar 3 2020, 11:22 AM

Fine with me, others commented so we should wait.

hbae added a comment.Mar 4 2020, 6:45 AM

Looks good to me.

This revision was not accepted when it landed; it landed in state Needs Review.Mar 4 2020, 7:20 AM
This revision was automatically updated to reflect the committed changes.