[PATCH] D64394: [MachineCSE][MachinePRE] Do not hoist common computations into loop bodies

[Kai Luo via Phabricator via llvm-commits]

lkail created this revision.
lkail added reviewers: hfinkel, jsji, steven.zhang, Jiangning, anton-afanasyev, ab.
Herald added subscribers: llvm-commits, MaskRay, hiraditya, nemanjai.
Herald added a project: LLVM.

Current PRE hoists common computations into
`CMBB = DT->findNearestCommonDominator(MBB, MBB1)`.
However, if `CMBB` is in a loop body, we might get performance
degradation.


Repository:
  rL LLVM

https://reviews.llvm.org/D64394

Files:
  llvm/lib/CodeGen/MachineCSE.cpp
  llvm/test/CodeGen/PowerPC/machine-pre.ll


Index: llvm/test/CodeGen/PowerPC/machine-pre.ll
===================================================================
--- llvm/test/CodeGen/PowerPC/machine-pre.ll
+++ llvm/test/CodeGen/PowerPC/machine-pre.ll
@@ -8,25 +8,25 @@
 ; CHECK-P9:       # %bb.0: # %entry
 ; CHECK-P9-NEXT:    lis r7, 0
 ; CHECK-P9-NEXT:    li r6, 0
+; CHECK-P9-NEXT:    li r8, 0
 ; CHECK-P9-NEXT:    li r9, 0
-; CHECK-P9-NEXT:    li r10, 0
 ; CHECK-P9-NEXT:    ori r7, r7, 65535
 ; CHECK-P9-NEXT:    .p2align 5
 ; CHECK-P9-NEXT:  .LBB0_1: # %header
 ; CHECK-P9-NEXT:    #
-; CHECK-P9-NEXT:    addi r10, r10, 1
-; CHECK-P9-NEXT:    cmpw r10, r3
-; CHECK-P9-NEXT:    addi r8, r5, 1024
+; CHECK-P9-NEXT:    addi r9, r9, 1
+; CHECK-P9-NEXT:    cmpw r9, r3
 ; CHECK-P9-NEXT:    blt cr0, .LBB0_4
 ; CHECK-P9-NEXT:  # %bb.2: # %cont
 ; CHECK-P9-NEXT:    #
-; CHECK-P9-NEXT:    add r9, r9, r4
-; CHECK-P9-NEXT:    cmpw r9, r7
+; CHECK-P9-NEXT:    add r8, r8, r4
+; CHECK-P9-NEXT:    cmpw r8, r7
 ; CHECK-P9-NEXT:    bgt cr0, .LBB0_1
 ; CHECK-P9-NEXT:  # %bb.3: # %cont.1
-; CHECK-P9-NEXT:    mr r6, r8
+; CHECK-P9-NEXT:    addi r6, r5, 1024
 ; CHECK-P9-NEXT:  .LBB0_4: # %return
-; CHECK-P9-NEXT:    mullw r3, r6, r8
+; CHECK-P9-NEXT:    addi r3, r5, 1024
+; CHECK-P9-NEXT:    mullw r3, r6, r3
 ; CHECK-P9-NEXT:    blr
 entry:
   br label %header
Index: llvm/lib/CodeGen/MachineCSE.cpp
===================================================================
--- llvm/lib/CodeGen/MachineCSE.cpp
+++ llvm/lib/CodeGen/MachineCSE.cpp
@@ -25,6 +25,7 @@
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineInstr.h"
+#include "llvm/CodeGen/MachineLoopInfo.h"
 #include "llvm/CodeGen/MachineOperand.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/Passes.h"
@@ -66,6 +67,7 @@
     AliasAnalysis *AA;
     MachineDominatorTree *DT;
     MachineRegisterInfo *MRI;
+    MachineLoopInfo *LI;
 
   public:
     static char ID; // Pass identification
@@ -83,6 +85,8 @@
       AU.addPreservedID(MachineLoopInfoID);
       AU.addRequired<MachineDominatorTree>();
       AU.addPreserved<MachineDominatorTree>();
+      AU.addRequired<MachineLoopInfo>();
+      AU.addPreserved<MachineLoopInfo>();
     }
 
     void releaseMemory() override {
@@ -802,6 +806,12 @@
     if (!CMBB->isLegalToHoistInto())
       continue;
 
+    // Don't hoist the instruction into a loop.
+    // FIXME: Can we go on looking for a non-loop ancester in the dominator
+    // tree?
+    if (LI->getLoopFor(CMBB) != nullptr)
+      continue;
+
     // Two instrs are partial redundant if their basic blocks are reachable
     // from one to another but one doesn't dominate another.
     if (CMBB != MBB1) {
@@ -863,6 +873,7 @@
   MRI = &MF.getRegInfo();
   AA = &getAnalysis<AAResultsWrapperPass>().getAAResults();
   DT = &getAnalysis<MachineDominatorTree>();
+  LI = &getAnalysis<MachineLoopInfo>();
   LookAheadLimit = TII->getMachineCSELookAheadLimit();
   bool ChangedPRE, ChangedCSE;
   ChangedPRE = PerformSimplePRE(DT);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D64394.208618.patch
Type: text/x-patch
Size: 3045 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190709/3d086c3b/attachment.bin>