[PATCH] RC->contains is a trap... (PR16106)

[Eric Christopher]

Hi Jakob and Lang,

RC->contains is a trap for the register allocator since it tests the
presence of the register in the class, however, it ignores any
additions or removals done by an AltOrder and so shouldn't be used in
this situation. The greedy allocator uses AllocationOrder, but the
fast allocator doesn't and just grabs the order specifically when it
needs it.

I see a few possible ways here:

a) the patch I've got,
b) modify contains,
c) use AllocationOrder in the fast allocator,
d) something else (fix the hint? something?)

At any rate contains should probably go away or something. It just
seems like a trap. I haven't audited all uses of contains throughout
the compiler, but this appears to be the only use in any of the
allocators.

Thoughts?

-eric
-------------- next part --------------
commit 88cc8b127f132864880d647371b5599f4c86abdb
Author: Eric Christopher <echristo at gmail.com>
Date:   Wed Jun 12 11:45:18 2013 -0700

    RC->contains is a trap for the register allocator since it tests the
    presence of the register in the class, however, it ignores any additions
    or removals done by an AltOrder and so shouldn't be used in this
    situation.
    
    Replace a use in the fast register allocator with a simple loop over
    the actual allocation order looking for a register hint to figure out
    if it's an allocatable register.
    
    Fixes PR16106.

diff --git a/lib/CodeGen/RegAllocFast.cpp b/lib/CodeGen/RegAllocFast.cpp
index bb9c05c..b068efe 100644
--- a/lib/CodeGen/RegAllocFast.cpp
+++ b/lib/CodeGen/RegAllocFast.cpp
@@ -516,12 +516,22 @@ RAFast::LiveRegMap::iterator RAFast::allocVirtReg(MachineInstr *MI,
          "Can only allocate virtual registers");
 
   const TargetRegisterClass *RC = MRI->getRegClass(VirtReg);
+  ArrayRef<MCPhysReg> AO = RegClassInfo.getOrder(RC);
 
   // Ignore invalid hints.
   if (Hint && (!TargetRegisterInfo::isPhysicalRegister(Hint) ||
-               !RC->contains(Hint) || !MRI->isAllocatable(Hint)))
+               !MRI->isAllocatable(Hint)))
     Hint = 0;
 
+  bool Found = false;
+  for (ArrayRef<MCPhysReg>::iterator I = AO.begin(), E = AO.end(); I != E; ++I) {
+    if (*I == Hint) {
+      Found = true;
+      break;
+    }
+  }
+  Hint = Found ? Hint : 0;
+
   // Take hint when possible.
   if (Hint) {
     // Ignore the hint if we would have to spill a dirty register.
@@ -531,12 +541,11 @@ RAFast::LiveRegMap::iterator RAFast::allocVirtReg(MachineInstr *MI,
         definePhysReg(MI, Hint, regFree);
       // definePhysReg may kill virtual registers and modify LiveVirtRegs.
       // That invalidates LRI, so run a new lookup for VirtReg.
+      DEBUG(dbgs() << "Using hint to assign:\n");
       return assignVirtToPhysReg(VirtReg, Hint);
     }
   }
 
-  ArrayRef<MCPhysReg> AO = RegClassInfo.getOrder(RC);
-
   // First try to find a completely free register.
   for (ArrayRef<MCPhysReg>::iterator I = AO.begin(), E = AO.end(); I != E; ++I){
     unsigned PhysReg = *I;
diff --git a/test/CodeGen/X86/x86-64-fastalloc-ah.ll b/test/CodeGen/X86/x86-64-fastalloc-ah.ll
new file mode 100644
index 0000000..73b4a68
--- /dev/null
+++ b/test/CodeGen/X86/x86-64-fastalloc-ah.ll
@@ -0,0 +1,23 @@
+; RUN: llc -mtriple=x86_64-pc-linux -O0 %s -o - | FileCheck %s
+
+; Check that we emit a copy out of %ah so that we can use it in a store
+; instruction later.
+; CHECK: movb    %ah, {{.*}}  # NOREX
+define void @main() {
+entry:
+  %a = call i8* @malloc(i32 2)
+  %acast = bitcast i8* %a to [0 x i8]*
+  %elemptr = getelementptr inbounds [0 x i8]* %acast, i32 0, i32 0
+
+  ; input is here (9)
+  %y = urem i8 9, 10
+
+  %z = add i8 48, %y
+  store i8 %z, i8* %elemptr
+
+  call void @printf(i8* %elemptr)
+  ret void
+}
+
+declare void @printf(i8*)
+declare i8* @malloc(i32)