[libclc] Move conversion builtins to the CLC library #124727
Conversation
|
CC @rjodinchr - are you able to test this PR out for your needs, by any chance? It's quite intrusive so I'd like to be extra sure I haven't broken anything. |
|
✅ With the latest revision this PR passed the Python code formatter. |
|
This is not correct for |
|
I think for |
Oh dear - thanks for trying out the PR so promptly. I fear that if clspv doesn't provide (e.g.) I think I may need to try out clspv for myself so I get a better idea of what's going wrong. |
|
In fact my setup was busted. In the end it seems to work without change. |
Thanks! Just for reference, is it just clspv's |
|
clspv's |
This is following llvm/llvm-project#124727, but it can land before the llvm change.
This is following llvm/llvm-project#124727, but it can land before the llvm change. Ref google#1447
This is following llvm/llvm-project#124727, but it can land before the llvm change. Ref google#1447
This is following llvm/llvm-project#124727, but it can land before the llvm change. Ref #1447
This commit moves the implementations of conversion builtins to the CLC library. It keeps the dichotomy of regular vs. clspv implementations of the conversions. However, for the sake of a consistent interface all CLC conversion routines are built, even the ones that clspv opts out of in the user-facing OpenCL layer. It simultaneously updates the python script to use f-strings for formatting.
frasercrmck
force-pushed
the
libclc-clc-conversions
branch
from
3945d49 to
585b1f7
Compare
|
I'm seeing several (82 out of 1045) conversions failures in the OpenCL-CTS. It's in saturating conversions from floating-point types to integers, and in regular float-to-float conversions. It isn't a result of this PR as they are already present on I will merge this PR and investigate the failures next. They should be easier to fix on top of this PR. I wonder if the clspv-specific conversions are more correct. |
This commit moves the implementations of conversion builtins to the CLC library. It keeps the dichotomy of regular vs. clspv implementations of the conversions. However, for the sake of a consistent interface all CLC conversion routines are built, even the ones that clspv opts out of in the user-facing OpenCL layer. It simultaneously updates the python script to use f-strings for formatting.
This commit moves the implementations of conversion builtins to the CLC library. It keeps the dichotomy of regular vs. clspv implementations of the conversions. However, for the sake of a consistent interface all CLC conversion routines are built, even the ones that clspv opts out of in the user-facing OpenCL layer. It simultaneously updates the python script to use f-strings for formatting.
|
This change broke standalone libclc builds on Gentoo: |
|
The same failure also happens when SPIRV targets are disabled. |
|
I'm guessing that both |
|
Ah, it's just incorrect dependency. Filed #127315 to fix it. |
In llvm#127378 it was reported that builds without clspv targets enabled were failing after llvm#124727, as all targets had a dependency on a file that only clspv targets generated. A quick fix was merged in llvm#127315 which wasn't correct. It moved the dependency on those generated files to the spirv targets, instead of onto the clspv targets. This means a build with spirv targets and without clspv targets would see the same problems as llvm#127378 reported. I tried simply removing the requirement to explicitly add dependencies to the custom command, relying instead on the file-level dependencies. This didn't seem reliable enough; in some cases on a Makefiles build, the clang command compiling (e.g.,) convert.cl would begin before the file was fully written. Instead, we keep the target-level dependency but automatically infer it based on the generated file name, to avoid manual book-keeping of pairs of files and targets. This commit also fixes what looks like an unintended bug where, when ENABLE_RUNTIME_SUBNORMAL was enabled, the OpenCL conversions weren't being compiled.
In #127378 it was reported that builds without clspv targets enabled were failing after #124727, as all targets had a dependency on a file that only clspv targets generated. A quick fix was merged in #127315 which wasn't correct. It moved the dependency on those generated files to the spirv targets, instead of onto the clspv targets. This means a build with spirv targets and without clspv targets would see the same problems as #127378 reported. I tried simply removing the requirement to explicitly add dependencies to the custom command, relying instead on the file-level dependencies. This didn't seem reliable enough; in some cases on a Makefiles build, the clang command compiling (e.g.,) convert.cl would begin before the file was fully written. Instead, we keep the target-level dependency but automatically infer it based on the generated file name, to avoid manual book-keeping of pairs of files and targets. This commit also fixes what looks like an unintended bug where, when ENABLE_RUNTIME_SUBNORMAL was enabled, the OpenCL conversions weren't being compiled.
This commit moves the implementations of conversion builtins to the CLC library. It keeps the dichotomy of regular vs. clspv implementations of the conversions. However, for the sake of a consistent interface all CLC conversion routines are built, even the ones that clspv opts out of in the user-facing OpenCL layer. It simultaneously updates the python script to use f-strings for formatting.
This commit moves the implementations of conversion builtins to the CLC
library. It keeps the dichotomy of regular vs. clspv implementations of
the conversions. However, for the sake of a consistent interface all CLC
conversion routines are built, even the ones that clspv opts out of in
the user-facing OpenCL layer.
It simultaneously updates the python script to use f-strings for
formatting.