Change usage of _PSTL_UD[R/S]_PRESENT macros

[adamfidel]

On some platforms, specifically Fedora 40 with GCC 14, the included pstl_config.h is such that the macros _PSTL_UDR_PRESENT and _PSTL_UDS_PRESENT are present but defined as an empty macro. This is causing compilation errors.

In this PR, we work around the empty macros by first testing _PSTL_UDR_PRESENT + 0, which evaluates to +0 (i.e., false) if the macro is undefined and the correct value if it is defined.

[danhoeflinger]

It seems this is a place it is defined like this:

https://github.com/gcc-mirror/gcc/blob/0695aba3e987f4bb06c95f29ff90a8a3234e1507/libstdc%2B%2B-v3/include/pstl/pstl_config.h#L127

Should we instead align our _PSTL_UDR_PRESENT with how others treat it (defined on undefined) and then check defined(_PSTL_UDR_PRESENT)?

We added our definition ~7 mo ago.

[danhoeflinger]

Alternatively, perhaps we can instead "process" an empty but defined _PSTL_UDR_PRESENT into some local version that we then use so that we can have this workaround only in one place.

[adamfidel]

@danhoeflinger: Should we instead align our _PSTL_UDR_PRESENT with how others treat it (defined on undefined) and then check defined(_PSTL_UDR_PRESENT)?

Are you suggesting that we change oneDPL's pstl_config.h to make this macro either undefined or empty defined, instead of always being defined 0 or 1 as it is currently? I'm not sure what the "correct" version is or how the upstream PSTL manages their pstl_config.h. I believe @rarutyun might have to weigh in on that.

@danhoeflinger: Alternatively, perhaps we can instead "process" an empty but defined _PSTL_UDR_PRESENT into some local version that we then use so that we can have this workaround only in one place.

That was my original thought, but I couldn't quite place where to put it.

It doesn't make sense to put it in pstl_config.h since that is where the issue arises (and there's the question of whether it will be included at all instead of the system's pstl_config.h). It also doesn't makes sense to put it into onedpl_config.h because there we define our own _ONEDPL_UDR_PRESENT which raises the question, why do we have two macros for the same thing? I don't have the historical context for that.

[danhoeflinger]

We do not include pstl_config.h anywhere in our own code. I think it is here to easier sync with upstream, but someone with more knowledge of the sync process can correct me there. This wouldn't be the place to create our own version, but I also don't know the appropriate place.

I'm realizing that my original suggestion to align with their handling doesn't work, because we may encounter either way of defining _PSTL_UDR_PRESENT from an external source. Looking at the blame, gcc-mirror/gcc@3162ca0 is where they made the switch away from our current approach to this one.

It seems we will have the same problem with _PSTL_UDS_PRESENT as well. I didn't notice any others, but its worth a look.

One option is to make a macro:
_ONEDPL_DEFINED_AND_NOT_ZERO(FLAG) which could be reused for something like this and have the workaround in one place. However, again I don't know where to put that.

[adamfidel]

@danhoeflinger One option is to make a macro: _ONEDPL_DEFINED_AND_NOT_ZERO(FLAG) which could be reused for something like this and have the workaround in one place. However, again I don't know where to put that.

I changed this PR to create a new _ONEDPL_DEFINED_AND_NOT_ZERO macro and use that instead. I think it made it much simpler.

[danhoeflinger]

I think this doesn't do what we are advertising by the name (and we want the semantics of the name I think).
_PSTL_UDS_PRESENT is defined to empty in the external std lib def when it is "true" and left undefined when it is "false".

This macro will convert that empty string to a "false" rather than a "true".

I think we want it to be "true" / "1" any time it is defined and not explicitly set to "0".

[adamfidel]

Yes, you are correct. This macro should be truthy if the input is defined and empty, but that is not the case here.

I made this godbolt to try to write a macro that has all of the properties that we want, but I cannot come up with a macro that satisfies everything. So perhaps the original version of the PR is the solution that we should go with for now.

[danhoeflinger]

Yeah, this is kind of an annoying case.

How about this:
https://godbolt.org/z/GY1qTjb5e

It would require basically converting the public pstl defines to our own local copy of the pstl defines (which would be "0" or "1"), then we could use our own copy as "normal".

Its possible that we could work it out so that our local _ONEDPL_UDS_PRESENT is our "local" copy, where we only attempt to define it ourselves if it is undefined.

[adamfidel]

After some offline discussion, @danhoeflinger and I arrived at a version of the macro that now correctly determines if the input define is either 1 or defined but empty.

[danhoeflinger]

extra space

[adamfidel]

Fixed.

[danhoeflinger]

I changed this PR to create a new _ONEDPL_DEFINED_AND_NOT_ZERO macro and use that instead. I think it made it much simpler.

I like it a lot better now after this switch. One real comment (above) to address from my perspective, but I also think this deserves a look from @rarutyun or someone else with better understanding of the upstream process before going forward.

[danhoeflinger]

LGTM, but I agree with the clang format suggestions, so I'd prefer to take those before merging.
Also, I still think it deserves a look from someone more familiar with upstream sync.

[danhoeflinger]

LGTM

[mmichel11]

LGTM