Skip to content

Fix clang 14 errors in philox when using b8 #3333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 20, 2022

Conversation

umar456
Copy link
Member

@umar456 umar456 commented Nov 18, 2022

The random number generator for b8 was producing incorrect results
on clang 14 due to loop unrolling. This was caused by some incorrect indexing
in the transform function for b8. This PR addresses this issue and fixes additional
issue when generating b8 numbers in all backends

Description

  • Fixes clang 14 test failures in ireduce_cpu
  • Fixes the way the ctr values are used when generating b8 RNG values

Changes to Users

Better and more consistent RNG for b8 values across backends.

Checklist

  • Rebased on latest master
  • Code compiles
  • Tests pass
  • Functions added to unified API
  • Functions documented

The random number generator for b8 was producing incorrect results on clang 14
due to loop unrolling. This commit addresses the underlying issue caused by
ineffective indexing on the b8 RNG and updates one ireduce test to use the
ASSERT_VEC_ARRAYS_EQ function
@umar456 umar456 force-pushed the clang_uchar_random_fix branch from 29d5ce1 to a29014a Compare November 19, 2022 03:37
Previously the b8 RNG was only using the lest significant bits for the RNG
this is probably okay but it made the CPU indexing difficult. This commit
ensures that the LSB of each of the 4 integers are used instead of only
the first integer
@umar456 umar456 force-pushed the clang_uchar_random_fix branch from a29014a to 87f738d Compare November 19, 2022 05:16
@umar456 umar456 merged commit d8900ea into arrayfire:master Nov 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants