search

KhronosGroup / SPIRV-LLVM-Translator

A tool and a library for bi-directional translation between SPIR-V and LLVM IR
Other
468 stars 209 forks source link

User defined/declared functions are wrongly identified as OpenCL builtins #2513

Open asudarsa opened 4 months ago

asudarsa commented 4 months ago

Existing checks in OCLIsBuiltin(...) function are not sufficient to identify OpenCL builtins correctly. There is a possibility that user can define/declare functions with names that could cause them to be identified as OpenCL builtins.

An LLVM IR test that showcases this issue has been added as part of an ongoing PR: https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/2512/files#diff-7bb994ec5258f2246431ab51bfff2ff1643b801fefdd7a498c666ba5c1480ac4

Without the fix in this PR (https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/2512), this test will fail.

The above PR addresses user-defined functions. However, we do not currently have a resolution for user-declared functions.

Thanks

asudarsa commented 4 months ago

A well-rounded solution is available in https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/2525

The root cause in this issue is that the OCLIsBuiltin identifies any function name that ‘might’ be a OCL builtin name. In his case, the demangled name was ‘atomic_fetch_and_add_uint32_explicit’ and this name was not found in the OCLSPIRVBuiltinMap. If we directly call OCLSPIRVBuiltinMap::map, it does the following:

  1. Call OCLSPIRVBuiltinMap::find(name, &B)
  2. Assert if the call fails
  3. Return B Though, OCLIsBuiltin is named as such, all it does is return an unmangled name if one exists, irrespective of whether it is OCL builtin or not. So, we need an extra check instead of directly calling OCLSPIRVBuiltinMap::map
  4. Call OCLSPIRVBuiltinMap::find(name, &B)
  5. Return early if the call fails
  6. Proceed processing with B

Thanks