kbobrovs

Results 28 comments of kbobrovs
trafficstars

> This is not a valid assumption. There are plenty of macros which are not all caps (such as errno, setjmp, etc), so while this might save you some hassle,...

Folks, I would appreciate if could you review the following parts: @rolandschulz: sycl/include/std/experimental/simd.hpp sycl/include/sycl/ext/oneapi/experimental/invoke_simd.hpp sycl/test/invoke_simd/invoke_simd.cpp @v-klochkov: llvm/lib/SYCLLowerIR/LowerInvokeSimd.cpp @asudarsa: llvm/tools/sycl-post-link/ModuleSplitter.cpp llvm/tools/sycl-post-link/ModuleSplitter.h

The llvm/test/SYCLLowerIR/ESIMD/lower_invoke_simd.ll failure will be fixed by test update (hopefully).

> They will only remove them if unused. Why do they incorrectly assume they are unused? Is it because you spit into SPMD and SIMD and the entry point into...

> 'm pretty sure the intention of SYCL_EXTERNAL (as defined by SYCL spec and therefore not in the context of invoke_simd) was never to give a function external linkage if...

We discussed the `linkonce_odr`/`SYCL_EXTERNAL` problem with @rolandschulz and others in more details. Key takeaways: - `SYCL_EXTERNAL` does **not** affect function linkage as defined by C++ rules - `linkonce_odr` can be...

The only failure is InvokeSimd/invoke_simd_smoke.cpp, which is fixed by https://github.com/intel/llvm-test-suite/pull/1146

/verify with https://github.com/intel/llvm-test-suite/pull/1146

@asudarsa, @cperkinsintel - I resolved outstanding comments with @rolandschulz and @v-klochkov. Please take a look.

> Only nitpicks. LGTM. @asudarsa, thanks - let me address your NIT comments in a separate PR to reduce the risk of conflicts for this big PR.