[llvm-commits] [PATCH] Fix PR15031

[Adhemerval Zanella]

On 01/24/2013 03:59 PM, Bill Schmidt wrote:
> This patch addresses bug 15031.
>
> The common code in the post-RA scheduler to break anti-dependencies on the
> critical path contained a flaw.  In the reported case, an anti-dependency
> between the overlapping registers %X4 and %R4 exists:
>
> 	%X29<def> = OR8 %X4, %X4
> 	%R4<def>, %X3<def,dead,tied3> = LBZU 1, %X3<kill,tied1>
>
> The unpatched code breaks the dependency by replacing %R4 and its uses
> with %R3, the first register on the available list.  However, %R3 and
> %X3 overlap, so this creates two overlapping definitions on the same
> instruction.
>
> The fix is straightforward, preventing selection of a register that
> overlaps any other defined register on the same instruction.
>
> The test case is reduced from the bug report, and verifies that we no
> longer produce "lbzu 3, 1(3)" when breaking this anti-dependency.
>
> Is this OK to commit?
>
> Thanks!

The patch looks ok, just two small suggestions:


       continue;
+    // If NewReg overlaps any of the forbidden registers, we can't use it.
+    bool Forbidden = false;
+    for (std::vector<unsigned>::iterator it = Forbid.begin();
+         it != Forbid.end(); ++it)
+      if (TRI->regsOverlap(NewReg, *it)) {
+        Forbidden = true;
+        break;
+      }
+    if (Forbidden) continue;
     return NewReg;
   }

Avoid to evaluate end() at very time, use something like:

for (std::vector<unsigned>::iterator it = Forbid.begin(), ite = Forbid.end();
     it != ite; ++it)

Also, since the std::vector is for unsigned and has a small size I think SmallVector
could be a better joice.