[PATCH] D23220: Make TwoAddressInstructionPass::rescheduleMIBelowKill subreg-aware

[Michael Kuperstein via llvm-commits]

mkuper created this revision.
mkuper added a reviewer: echristo.
mkuper added a subscriber: llvm-commits.
Herald added a subscriber: mehdi_amini.

This fixes PR28824

I'm mostly wondering if we already have something like regOverlapsSet() around.
I couldn't find one, but it sounds like it ought to exist.

(The registers in the test are explicit on purpose - for the mov, ecx is ABI-mandated, and for the shift, it's a special instruction form that we want to be using.)

https://reviews.llvm.org/D23220

Files:
  lib/CodeGen/TwoAddressInstructionPass.cpp
  test/CodeGen/X86/pr28824.ll

Index: test/CodeGen/X86/pr28824.ll
===================================================================
--- test/CodeGen/X86/pr28824.ll
+++ test/CodeGen/X86/pr28824.ll
@@ -0,0 +1,23 @@
+; RUN: llc < %s -mtriple=i386-unknown-linux-gnu | FileCheck %s
+
+ at d = global i32 0, align 4
+
+; Verify the sar happens before ecx is clobbered with the parameter being
+; passed to fn3
+; CHECK-LABEL: fn4
+; CHECK: movb d, %cl
+; CHECK: sarl %cl
+; CHECK: movl $2, %ecx
+define i32 @fn4(i32 %i) #0 {
+entry:
+  %0 = load i32, i32* @d, align 4
+  %shr = ashr i32 %i, %0
+  tail call fastcc void @fn3(i32 2, i32 5, i32 %shr, i32 %i)
+  %cmp = icmp slt i32 %shr, 1
+  %. = zext i1 %cmp to i32
+  ret i32 %.
+}
+
+declare void @fn3(i32 %p1, i32 %p2, i32 %p3, i32 %p4) #0
+
+attributes #0 = { nounwind }
Index: lib/CodeGen/TwoAddressInstructionPass.cpp
===================================================================
--- lib/CodeGen/TwoAddressInstructionPass.cpp
+++ lib/CodeGen/TwoAddressInstructionPass.cpp
@@ -29,7 +29,7 @@
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/CodeGen/LiveIntervalAnalysis.h"
@@ -539,6 +539,16 @@
   return TRI->regsOverlap(RegA, RegB);
 }
 
+// Returns true if Reg is equal or aliased to at least one register in Set.
+static bool regOverlapsSet(const SmallSetVector<unsigned, 2> &Set, unsigned Reg,
+                           const TargetRegisterInfo *TRI) {
+  for(unsigned R : Set)
+    if (TRI->regsOverlap(R, Reg))
+      return true;
+
+  return false;
+}
+
 /// Return true if it's potentially profitable to commute the two-address
 /// instruction that's being processed.
 bool
@@ -864,9 +874,9 @@
     // FIXME: Needs more sophisticated heuristics.
     return false;
 
-  SmallSet<unsigned, 2> Uses;
-  SmallSet<unsigned, 2> Kills;
-  SmallSet<unsigned, 2> Defs;
+  SmallSetVector<unsigned, 2> Uses;
+  SmallSetVector<unsigned, 2> Kills;
+  SmallSetVector<unsigned, 2> Defs;
   for (const MachineOperand &MO : MI->operands()) {
     if (!MO.isReg())
       continue;
@@ -915,21 +925,21 @@
       if (!MOReg)
         continue;
       if (MO.isDef()) {
-        if (Uses.count(MOReg))
+        if (regOverlapsSet(Uses, MOReg, TRI))
           // Physical register use would be clobbered.
           return false;
-        if (!MO.isDead() && Defs.count(MOReg))
+        if (!MO.isDead() && regOverlapsSet(Defs, MOReg, TRI))
           // May clobber a physical register def.
           // FIXME: This may be too conservative. It's ok if the instruction
           // is sunken completely below the use.
           return false;
       } else {
-        if (Defs.count(MOReg))
+        if (regOverlapsSet(Defs, MOReg, TRI))
           return false;
         bool isKill =
             MO.isKill() || (LIS && isPlainlyKilled(&OtherMI, MOReg, LIS));
-        if (MOReg != Reg &&
-            ((isKill && Uses.count(MOReg)) || Kills.count(MOReg)))
+        if (MOReg != Reg && ((isKill && regOverlapsSet(Uses, MOReg, TRI)) ||
+                             regOverlapsSet(Kills, MOReg, TRI)))
           // Don't want to extend other live ranges and update kills.
           return false;
         if (MOReg == Reg && !isKill)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D23220.66990.patch
Type: text/x-patch
Size: 3325 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160805/4aea9dbe/attachment.bin>