Skip to content
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

Optimized sample generation in Sobol sequence code #593

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sshudler
Copy link

@sshudler sshudler commented Jul 19, 2022

This code does two things:

  • Sets the denormal mode and flush to zero mode (intrinsics API) to improve the performance of Embree (as per Embree recommendation).
  • Optimizes the generation of samples in Sobol sequence code. The original code compiled with MSVC++ had an extra load instruction in the loop, which affected branch prediction performance and lowered the IPC. Also, the loop was manually unrolled.

We noticed a performance improvement of about 6%-15% in some scenarios on some systems.

@csakthiv-intel

Comment on lines +52 to +59
result ^= ((i & 1) * directions[offset]);
i >>= 1; offset++;
result ^= ((i & 1) * directions[offset]);
i >>= 1; offset++;
result ^= ((i & 1) * directions[offset]);
i >>= 1; offset++;
result ^= ((i & 1) * directions[offset]);
i >>= 1; offset++;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no longer a bounds check on i for 3 of the 4 indexes into directions before the loop repeats, how do you ensure the offset is always a valid index?

Copy link
Author

@sshudler sshudler Oct 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before the change and after the change i wasn't used as an index into directions. Whenever the loop was hitting a 1 bit in i it would index into directions by that bit's offset. The way directions defined, starting from the offset it has SOBOL_BITS range for access, so in the new code the access to directions is valid and when i becomes 0 none of these directions values will be used in the result because of & with (0 & 1). Also, SOBOL_BITS == sizeof(i).

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