Vector registers are zero initialized by the kernel. Stop accepting "all ones" as a clean value.
Note that this was not working as expected given that value == 0xff can be assumed to be always false by the compiler as value's range is [-128, 127]. Both GCC (-Wtype-limits) and clang (-Wtautological-constant-out-of-range-compare) warn about this.
Signed-off-by: Ignacio Encinas ignacio@iencinas.com --- I tried looking why "all ones" was previously deemed a "clean" value but couldn't find any information. It looks like the kernel always zero-initializes the vector registers.
If "all ones" is still acceptable for any reason, my intention is to spin a v2 changing the types of `value` and `prev_value` to unsigned char. --- tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c b/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c index 35c0812e32de0c82a54f84bd52c4272507121e35..b712c4d258a6cb045aa96de4a75299714866f5e6 100644 --- a/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c +++ b/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c @@ -6,7 +6,7 @@ * the values. To further ensure consistency, this file is compiled without * libc and without auto-vectorization. * - * To be "clean" all values must be either all ones or all zeroes. + * To be "clean" all values must be all zeroes. */
#define __stringify_1(x...) #x @@ -46,7 +46,7 @@ int main(int argc, char **argv) : "=r" (value)); \ if (first) { \ first = 0; \ - } else if (value != prev_value || !(value == 0x00 || value == 0xff)) { \ + } else if (value != prev_value || value != 0x00) { \ printf("Register " __stringify(register) \ " values not clean! value: %u\n", value); \ exit(-1); \
--- base-commit: 03d38806a902b36bf364cae8de6f1183c0a35a67 change-id: 20250301-fix-v_exec_initval_nolibc-498d976c372d
Best regards,
On Wed, Mar 05, 2025 at 05:39:28PM +0100, Ignacio Encinas wrote:
Vector registers are zero initialized by the kernel. Stop accepting "all ones" as a clean value.
Note that this was not working as expected given that value == 0xff can be assumed to be always false by the compiler as value's range is [-128, 127]. Both GCC (-Wtype-limits) and clang (-Wtautological-constant-out-of-range-compare) warn about this.
This check was included because the "dirty" value is an implementation detail that I believe is not strongly defined in the ABI. Since linux does always set this value to zero (currently) we can safely remove this check.
Reviewed-by: Charlie Jenkins charlie@rivosinc.com Tested-by: Charlie Jenkins charlie@rivosinc.com
Signed-off-by: Ignacio Encinas ignacio@iencinas.com
I tried looking why "all ones" was previously deemed a "clean" value but couldn't find any information. It looks like the kernel always zero-initializes the vector registers.
If "all ones" is still acceptable for any reason, my intention is to spin a v2 changing the types of `value` and `prev_value` to unsigned char.
tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c b/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c index 35c0812e32de0c82a54f84bd52c4272507121e35..b712c4d258a6cb045aa96de4a75299714866f5e6 100644 --- a/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c +++ b/tools/testing/selftests/riscv/vector/v_exec_initval_nolibc.c @@ -6,7 +6,7 @@
- the values. To further ensure consistency, this file is compiled without
- libc and without auto-vectorization.
- To be "clean" all values must be either all ones or all zeroes.
*/
- To be "clean" all values must be all zeroes.
#define __stringify_1(x...) #x @@ -46,7 +46,7 @@ int main(int argc, char **argv) : "=r" (value)); \ if (first) { \ first = 0; \
} else if (value != prev_value || !(value == 0x00 || value == 0xff)) { \
} else if (value != prev_value || value != 0x00) { \ printf("Register " __stringify(register) \ " values not clean! value: %u\n", value); \ exit(-1); \
base-commit: 03d38806a902b36bf364cae8de6f1183c0a35a67 change-id: 20250301-fix-v_exec_initval_nolibc-498d976c372d
Best regards,
Ignacio Encinas ignacio@iencinas.com
On 5/3/25 22:49, Charlie Jenkins wrote:
On Wed, Mar 05, 2025 at 05:39:28PM +0100, Ignacio Encinas wrote:
Vector registers are zero initialized by the kernel. Stop accepting "all ones" as a clean value.
Note that this was not working as expected given that value == 0xff can be assumed to be always false by the compiler as value's range is [-128, 127]. Both GCC (-Wtype-limits) and clang (-Wtautological-constant-out-of-range-compare) warn about this.
This check was included because the "dirty" value is an implementation detail that I believe is not strongly defined in the ABI. Since linux does always set this value to zero (currently) we can safely remove this check.
Thanks for the review. Just after sending the patch I noticed it should also remove some code that becomes useless after this change: _prev_value_ and _first_ variables were only needed because two "clean" values were supported.
I'll send a v2 tomorrow. I'm guessing keeping your "Reviewed-by" and "Tested-by" is the appropriate thing to do as the changes are very simple. Let me know if that's not the case.
Thanks again!
On Thu, Mar 06, 2025 at 07:31:22AM +0100, Ignacio Encinas Rubio wrote:
On 5/3/25 22:49, Charlie Jenkins wrote:
On Wed, Mar 05, 2025 at 05:39:28PM +0100, Ignacio Encinas wrote:
Vector registers are zero initialized by the kernel. Stop accepting "all ones" as a clean value.
Note that this was not working as expected given that value == 0xff can be assumed to be always false by the compiler as value's range is [-128, 127]. Both GCC (-Wtype-limits) and clang (-Wtautological-constant-out-of-range-compare) warn about this.
This check was included because the "dirty" value is an implementation detail that I believe is not strongly defined in the ABI. Since linux does always set this value to zero (currently) we can safely remove this check.
Thanks for the review. Just after sending the patch I noticed it should also remove some code that becomes useless after this change: _prev_value_ and _first_ variables were only needed because two "clean" values were supported.
I'll send a v2 tomorrow. I'm guessing keeping your "Reviewed-by" and "Tested-by" is the appropriate thing to do as the changes are very simple. Let me know if that's not the case.
Thanks again!
Yes, those changes seem small so you can keep the tags :)
- Charlie
linux-kselftest-mirror@lists.linaro.org
-
Charlie Jenkins -
Ignacio Encinas -
Ignacio Encinas Rubio