Some cosmetic comments below, otherwise the patches are: reviewed-by: Vincent Lejeune <vljn at ovi.com>
>- OutStreamer.EmitRawText( >- Twine("; Kernel info:\n") + >- "; NumSgprs: " + Twine(KernelInfo.NumSGPR) + "\n" + >- "; NumVgprs: " + Twine(KernelInfo.NumVGPR) + "\n"); >+ if (STM.getGeneration() > AMDGPUSubtarget::NORTHERN_ISLANDS) { >+ I think it would look cleaner without empty newline here >+ OutStreamer.EmitRawText( >+ Twine("; Kernel info:\n") + >+ "; NumSgprs: " + Twine(KernelInfo.NumSGPR) + "\n" + >+ "; NumVgprs: " + Twine(KernelInfo.NumVGPR) + "\n"); >+ } else { >+void CFStack::pushBranch(unsigned Opcode, bool isWQM) { >+ CFStack::StackItem Item = CFStack::ENTRY; >+ switch(Opcode) { >+ case AMDGPU::CF_PUSH_EG: >+ case AMDGPU::CF_ALU_PUSH_BEFORE: >+ if (!isWQM) { >+ if (!ST.hasCaymanISA() && >!branchStackContains(CFStack::FIRST_NON_WQM_PUSH)) >+ Item = CFStack::FIRST_NON_WQM_PUSH; // May not be required on >Evergreen/NI >+ // See comment in >+ // CFStack::getSubEntrySize() >+ else if (CurrentEntries > 0 && >+ ST.getGeneration() > AMDGPUSubtarget::EVERGREEN && >+ !ST.hasCaymanISA() && >+ !branchStackContains(CFStack::FIRST_NON_WQM_PUSH_W_FULL_ENTRY)) >+ Item = CFStack::FIRST_NON_WQM_PUSH_W_FULL_ENTRY; >+ else >+ Item = CFStack::SUB_ENTRY; >+ } else { >+ Item = CFStack::ENTRY; It's a single line statement, I think it should be without brace. >+ } >+ break; > case AMDGPU::CF_ALU_PUSH_BEFORE: >- CurrentStack++; >- MaxStack = std::max(MaxStack, CurrentStack); >- HasPush = true; >- if (ST.hasCaymanISA() && CurrentLoopDepth > 1) { >+ if (ST.hasCaymanISA() && CFStack.getLoopDepth() > 1) { > BuildMI(MBB, MI, MBB.findDebugLoc(MI), > TII->get(AMDGPU::CF_PUSH_EG)) > .addImm(CfCount + 1) > .addImm(1); > MI->setDesc(TII->get(AMDGPU::CF_ALU)); > CfCount++; >+ CFStack.pushBranch(AMDGPU::CF_PUSH_EG); >+ } else { >+ CFStack.pushBranch(AMDGPU::CF_ALU_PUSH_BEFORE); Here too > } >+bool CFStack::requiresWorkAroundForInst(unsigned Opcode) { >+ if (Opcode == AMDGPU::CF_ALU_PUSH_BEFORE && ST.hasCaymanISA() && >+ getLoopDepth() > 1) { >+ return true; And here too >+ } Thank for this patch set, stack bugs are really not easy to spot and fix. Vincent > Le Mercredi 11 décembre 2013 19h07, Tom Stellard <t...@stellard.net> a écrit : > > Hi, > > The attached patches implement a work-around for the CF stack HW bug > that is present on some Evergreen and NI GPUs. > > Please Review. > > -Tom > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev