[Mesa-dev] PATCHES: R600: Implement work-around for CF stack HW bug

[Vincent Lejeune]

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 <tom at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>