[SYCL] Fix compilation fail on Windows

[aelovikov-intel]

Apparently, std::max(smth) cannot be used...

[bader]

Apparently, std::max(smth) cannot be used...

I see a lot of std::max(smth) uses in the project. Could you clarify the problem with using std::max here?

[aelovikov-intel]

Apparently, std::max(smth) cannot be used...

I see a lot of std::max(smth) uses in the project. Could you clarify the problem with using std::max here?

I believe it depends on what headers are included prior to this specific include by the customer's code. It seems that this particular test has some "bad" system include resulting in the following after the preprocessor:

#pragma warning(pop)
# 183 "C:\\Program Files (x86)\\Windows Kits\\10\\\\include\\10.0.22000.0\\\\shared\\minwindef.h" 2 3


/* Types use for passing & returning polymorphic values */
typedef UINT_PTR WPARAM;
typedef LONG_PTR LPARAM;
typedef LONG_PTR LRESULT;




#define max(a,b) (((a) > (b)) ? (a) : (b))



#define min(a,b) (((a) < (b)) ? (a) : (b))




#define MAKEWORD(a,b) ((WORD)(((BYTE)(((DWORD_PTR)(a)) & 0xff)) | ((WORD)((BYTE)(((DWORD_PTR)(b)) & 0xff))) << 8))
#define MAKELONG(a,b) ((LONG)(((WORD)(((DWORD_PTR)(a)) & 0xffff)) | ((DWORD)((WORD)(((DWORD_PTR)(b)) & 0xffff))) << 16))
...
  constexpr size_t getAlignment() const {
    return std::(((alignof(T)) > (Alignment)) ? (alignof(T)) : (Alignment));
  }

[smaslov-intel]

any quick note how this resolves the build issue? should we move the solution to a common header file?

[bader]

FYI: e7d4537

[smaslov-intel]

FYI: e7d4537

Hmm, why is that "fix" not enough?

[bader]

There are a few ways to handle this problem.
I suppose the issue can be exposed if someone explicitly include this header and re-define max symbol with a macro.
e7d4537 shows one of the approaches to avoid this problem. Codeplay folks applied a bit different solution in the barrier extension header - 2e3f61e.

BTW, I see a few more uses of std::max in sycl/include/CL/sycl/usm.hpp which probably should be updated as well. I wonder how these are passing sycl/test/basic_tests/min_max_test.cpp test. Maybe -fsycl-device-only has something to do with it....

[aelovikov-intel]

I think I'll be able to remove the using directive here and it would still work. Awaiting local build. Also, I don't need the std::max, a max would suffice just as well. And I'd expect that we'll have either a macro or sycl::max inside current scope at this point.

[bader]

I think I'll be able to remove the using directive here and it would still work.

This reverts the change. So, how did you manage to get the compilation fail?

[aelovikov-intel]

I'll remove the explicit std::, it will be simply return max(...).

[aelovikov-intel]

I believe the failure in the "Build + LIT" task is unrelated to this PR and should be ignored.

Also: #6243 (review)

[pvchupin]

I believe the failure in the "Build + LIT" task is unrelated to this PR and should be ignored.

Also: #6243 (review)

Restarted testing after problematic commit was reverted.

[aelovikov-intel]

Ready to merge I believe.