parallel-runtimes / lomp

Little OpenMP Library
Apache License 2.0
153 stars 17 forks source link

Implement (basic) OMP_DISPLAY_ENV envirable and omp_display_env() API routine #51

Closed mjklemm closed 2 years ago

mjklemm commented 2 years ago

@JimCownie Ping :-)

JimCownie commented 2 years ago

I'll look tomorrow morning.

-- Jim +44 780 637 7146

On Tue, 22 Feb 2022, 17:15 Michael Klemm, @.***> wrote:

@JimCownie https://github.com/JimCownie Ping :-)

— Reply to this email directly, view it on GitHub https://github.com/parallel-runtimes/lomp/pull/51#issuecomment-1048025332, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALY7TVBIJSJBTNKKLOFN4BDU4PAC3ANCNFSM5JRLFBUA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

mjklemm commented 2 years ago

OMP_DISPLAY_ENV is a ternary variable, I wanted to keep that logic in the source. I could use an enum class instead to be even more explicit.

JimCownie commented 2 years ago

So where does the bool come from?

Thinking about this whole thing some more, wouldn't it be better simply to scan the whole environment for relevant envirables (ones which start OMP or LOMP) and then print them, rather than having a specific list. If you want to flag ones which LOMP does not support, you would still need to have a list of the ones we support (better than ones we don’t, since the standard may change and add new ones) and flag any others in the OMP_ space which aren’t in that list.

On 28 Feb 2022, at 11:43, Michael Klemm @.***> wrote:

OMP_DISPLAY_ENV is a ternary variable, I wanted to keep that logic in the source. I could use an enum class instead to be even more explicit.

— Reply to this email directly, view it on GitHub https://github.com/parallel-runtimes/lomp/pull/51#issuecomment-1054172236, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALY7TVAMRIT4WPOG2JKBQBLU5NNWTANCNFSM5JRLFBUA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you were mentioned.

-- Jim James Cownie @.***> Mob: +44 780 637 7146

mjklemm commented 2 years ago

The weird thing is: OMP_DISPLAY_ENV is ternary (false, true, verbose) whereas for omp_display_env the value false is encoded by not calling the function and the argument then being a "bool" that either turns on verbose printing or no printing.

Scanning the environment won't work, as OMP_DISPLAY_ENV and omp_display_env are supposed to print all ICVs (and their envirables) regardless of whether they are actually set in the environment.

mjklemm commented 2 years ago

Let me make a quick update and replace the magic constants 1 and 2 with enum-class values. That seems better the more I ponder about this.