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

Prevent offset access with block arrays to thwart dynamic ALA #25933

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

e-kayrakli
Copy link
Contributor

@e-kayrakli e-kayrakli commented Sep 11, 2024

This PR makes sure that ALA is applicable for:

var A = blockDist.createArray(...)
const InnerDomain = A.domain.expand(-1);

forall i in InnerDomain {
  A[i] = A[i-1];
}

Things of note:

  • this is a dynamically optimized loop, because the compiler can't statically determine that InnerDomain and A.domain are aligned.
  • the body contains both regular (A[i]) and offset (A[i-1]) access, where the latter is not optimizable for a block-distributed array

The problem is, #25712 modified the dynamic checks in scenarios like this. After that PR, the generated optimization looked like

param staticRegularFlag = alaStaticallySupported(A);  // added for A[i] (true)
param staticOffsetFlag = alaOffsetStaticallySupported(A);  // added for A[i-1] (false)

if ( (!staticRegularFlag || alaDynamicallySupported(A)) &&
                            alaOffsetCheck(A, 1)) {
  // optimized loop
}
else {
  // unoptimized loop
}

Note that ALA has static override (!staticRegularFlag || part) which allows optimization and the dynamic checks to be reverted in case a static check fails. This allows some accesses to be optimized while some others fails. However, the same override is not there for the offset check. This PR adds a similar !staticOffsetFlag || for similar purposes. This allows A[i] to be optimized as before while A[i-1] is not.

Test:

  • performance goes back to where we were before
  • gasnet
  • linux64

@e-kayrakli e-kayrakli merged commit e4c1b6f into chapel-lang:main Sep 11, 2024
7 checks passed
@e-kayrakli e-kayrakli deleted the ala-block-with-offset branch September 11, 2024 23:45
stonea added a commit that referenced this pull request Sep 25, 2024
This adds the following annotations to the stencil 2d graph (would be
nice to put in release notes):

- #25712 went in on 8/13 which
caused the StencilDist line to meet the StencilDist_localAccess line
- #25701 went in on 8/19 which
improved the performance of both StencilDist versions
- #25933 when in on 9/11,
fixing the slight performance regression in the BlockDist version

[reviewed-by: nobody; annotations update]
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