[ESIMD] Cast argument for atomic updates to integral type

[fineg74]

Complementary test PR intel/llvm-test-suite#1407

[kbobrovs]

• what is the type combination in the failing case?
• the corresponding test update does not seem to add any new test case

[fineg74]

providing float as an arguments breaks the compilation as _MsgT is an integral type (int32_t, uint32_t, int64_t, uint64_t) and it requires an explicit type conversion from non-integral type.
Corresponding test fails compilation without this change as it uses float type as an argument

[fineg74]

The test that is actually failing is SYCL/ESIMD/lsc/atomic_cmpxchg.cpp (after the fix)

[v-klochkov]

Sorry, but I don't understand this change-set. If atomic for float value causes a compilation, then why convert is the proper solution?
Let's say the argument if float32 values 2.3, then convert to uint32_t would change 2.3 to 2.0 right and then to uint32_t value 2, right?

[kbobrovs]

providing float as an arguments breaks the compilation

there is no legal user-level atomic API invocation which would translate into __esimd_lsc_xatomic_stateless_1 with floating-point argument. So I believe compilation failure is the right behavior.

[v-klochkov]

The additional fix replaced 'convert' with 'bit_cast'. Please answer the question asked above. How is that possible to have FP types in __esimd_lsc_xatomic_stateless_1()?
Why the bit_cast is needed here?

[fineg74]

Per sycl/ext/intel/esimd/common.hpp get_num_args() fadd, fsub, fmin, fmax have 1 argument
Per sycl/ext/intel/esimd/memory.hpp check_atomic fadd, fsub, fmin, fmax have to accept either float or half types
So in case of these operator calls __esimd_lsc_xatomic_stateless_1() will be called with FP types as arguments.
The bitcast is required since __esimd_lsc_xatomic_stateless_1() accepts only integral type vector as an argument so FP data should be passed as integral data. The cast<> call may lead to loss of data bitcast will preserve the data while passing argument to intrinsic

[v-klochkov]

Ok, this explanation helped. Thank you.
So, fadd is 2 arguments, but here it is considered as 1 argument because the other/memory operand does not count and this function being updated here accepts fp operations like fadd correctly.

[v-klochkov]

Please add a test for the problem being fixed here.
This PR intel/llvm-test-suite#1407 does not test it because the test passed even without this PR (#7495)

[v-klochkov]

Please add a test for the problem being fixed here. This PR intel/llvm-test-suite#1407 does not test it because the test passed even without this PR (#7495)

Actually..., sorry, it is my mistake. The test did not pass without this PR because that is a PVC test and we don't have PVC in CI.

[v-klochkov]

Before merging this fix, please check/confirm that the corresponding LIT test runs successfully on PVC (i.e. not only the compilation phase passes).

BTW, can you please update that LIT test once again?
I am asking because the previous version of the fix here had 'convert' instead of 'bitcast', which was incorrect as FP data had to lose/trunc precision. If you previously claimed that it passed with 'convert', then the FP values used in the test are like 2.0, 3.0, etc, instead of something like 2.1, 3.14, etc.

[fineg74]

Before merging this fix, please check/confirm that the corresponding LIT test runs successfully on PVC (i.e. not only the compilation phase passes).

BTW, can you please update that LIT test once again? I am asking because the previous version of the fix here had 'convert' instead of 'bitcast', which was incorrect as FP data had to lose/trunc precision. If you previously claimed that it passed with 'convert', then the FP values used in the test are like 2.0, 3.0, etc, instead of something like 2.1, 3.14, etc.

Added PR for updated test: intel/llvm-test-suite#1458
Confirmed it works on PVC