[PATCH]: Bug 18747 - Add an option to allow an exhaustive search during last chance recoloring

[Quentin Colombet]

Hi Mayur,

Thanks for working on this.

The general structure looks good to me, but we have to make a few fixes:
#1
+    // Both lcr-max-depth and lcr-max-interf encountered
+    CO_Depth_Interf
+  };
You do not need this one, just assign 0 to None, 1 to Depth, and 2 to Interf.
Then, we will use them as bit position in a byte.

#2
+  struct CutoffInfo {
+    CutOffStage Stage;
+    void setCutOffInfo(CutOffStage state = CO_None) {
+      Stage = state;
+    }
+  }CO_Info;

CO_Info can simply be an uint8_t.
Note: if you want to hide the bit masking tricks, you can encapsulate those in the struct you’d proposed.

#2.1 BTW, I am not sure using _ in a variable name is common practice in the LLVM code base.

This will solve this bug:
       DEBUG(dbgs() << "Early abort: too many interferences.\n");
+      CO_Info.setCutOffInfo(CO_Interf);
       return false;
 At this point, this is possible that the depth case was encountered during the same session, thus this setting is not accurate and we would have to play the same if/else trick as you did for the depth case.

#3
With the new proposal, this simply becomes:
CO_Info |= CO_Interf

#4
Likewise, this code sequence:
+    // set the cutoffstage
+    if (CO_Info.Stage == CO_Interf)
+      CO_Info.setCutOffInfo(CO_Depth_Interf);
+    else
+      CO_Info.setCutOffInfo(CO_Depth);
becomes:
CO_Info |= CO_Depth

+  CO_Info.setCutOffInfo();
=> CO_Info = CO_None;

#5
+  if (Reg == ~0U && (CO_Info.Stage != CO_None)) {
+    if (CO_Info.Stage == CO_Depth)
+      report_fatal_error("register allocation failed: maximum depth for recoloring reached");
+    else if (CO_Info.Stage == CO_Interf)
+      report_fatal_error("register allocation failed: maximum interference for recoloring reached");
+    else
+      report_fatal_error("register allocation failed: maximum interference and depth for recoloring reached");
+  }
=>
+  if (Reg == ~0U && (CO_Info != CO_None)) {
+    uint8_t COEncountered = CO_Info & (CO_Depth | CO_Interf);
+    if (COEncountered == CO_Depth)
+      report_fatal_error("register allocation failed: maximum depth for recoloring reached");
+    else if (COEncountered == CO_Interf)
+      report_fatal_error("register allocation failed: maximum interference for recoloring reached");
+    else if (COEncountered == (CO_Depth | CO_Interf))
+      report_fatal_error("register allocation failed: maximum interference and depth for recoloring reached");
+  }

#6
Instead of using report_fatal_error, use LLVMContext::diagnose or LLVMContext::emitError. You should be able to access the LLVMContext through the MF field (maybe with one indirection: getFunction).

#7
The error messages may be more specific.
I admit I do not have a good idea now for the exact messages, but mentioning last chance recoloring and the limits used for the related cut-off (maybe the name of the related option and its actual value) may help to decide what to do.
That said, this could be improved later and is not a blocker for your contribution :).

Thanks again!
-Quentin

On Mar 28, 2014, at 2:14 AM, MAYUR PANDEY <mayur.p at samsung.com> wrote:

> 
> Hi,
> 
> Attached is the patch for solving the 1st thing in the Bug 18747:
> 1. When the assignment fails because of the early cutoffs, this should be reported instead of the current obscur “ran out of registers”.
> the discussion can be checked at http://llvm.org/bugs/show_bug.cgi?id=18747
> 
> Please review the patch.
> 
> Thanks,
> Mayur<recoloringcutoff.patch>