Fix non-transposed Crout L1 offset - #34
Open
kerwtsai-amd wants to merge 1 commit into
Open
Conversation
HPL_pdpancrN() offsets its L1 base pointer by ICOFF * PANEL->jb, which is the strided shift the transposed variants need. In the non-transposed layout the kernel indexes L1 as L1[(JJ + j) * NB + i], so the JJ argument the launcher already passes supplies the strided shift itself; the base pointer must supply the shift on the contiguous axis, + ICOFF. The recursion driver HPL_pdrpancrN() hands the leaf the diagonal sub-block of L1 at Mptr(L1, ICOFF, ICOFF, PANEL->jb), i.e. L1 + ICOFF + ICOFF * jb. The leaf splits that between the base pointer and JJ, so applying the strided shift in both places and never applying the contiguous one walks off the end of L1, which holds exactly nb * nb doubles (PANEL->L1 = A0 + ml2 * JB and PANEL->dipiv = PANEL->L1 + JB * JB). The maximum reachable offset is in bounds only for ICOFF == 0. The symptom is a GPU memory access fault at any NB whenever the recursion produces a leaf with ICOFF >= 1, which is essentially always -- including NB=896, the value the README recommends for MI355X. The fault does not abort the MPI job, so an affected run hangs rather than exiting. HPL_pdpanllN() and HPL_pdpanrlN() already use L1 + jj; only the Crout variant carried the transposed form.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
L1by its column offset instead of a transposed-layout row stride.ICOFFfrom accessing beyond theNB * NBL1allocation.Root cause
HPL_pdpancrN()is the Crout leaf panel factorization for non-transposedL1. Its launcher usedL1 + jj * PANEL->jb, copied from the transposed variant. Transposed layout usesrow + column * NB, whereICOFF * NBis correct; non-transposed layout usescolumn + row * NB, so the contiguous base shift is onlyICOFF(jj). The kernel body already uses N-layout indexing such asL1[t * NB + jj].For recursive leaves with
ICOFF > 0, the old base can make the decisive store reach(ICOFF + t) * NB + j, beyond theNB * NBallocation. BecausedipivfollowsL1, this can corrupt pivots or fault directly.Reproduction
A full-size MI355X/gfx950 run with
N=540672,NB=1024,P=2,Q=4,PFACT=Crout,RFACT=Crout,NBMIN=4,NDIV=2, andL1=no-transposedfaults before producing a WR result or residual:PFACT=1withL1=1selects this leaf;RFACTcontrols the recursive driver but does not itself selectpdpancrN.Test plan
llN,llT,crN,crT,rlN,rlT) at NB=896 and NB=1024 on MI355X/gfx950; all 12 residual checks PASSED.