[ConstHoisting] choose to hoist when frequency is the same

[wmi]

Summary

The patch is to adjust the strategy of frequency based consthoisting: Previously when the candidate block has the same frequency with the existing blocks containing a const, it will not hoist the const to the candidate block. For that case, now we change the strategy to hoist the const if only existing blocks have more than one block member. This is helpful for reducing code size.

[davidxl]

lgtm

test/CodeGen/X86/constant-hoisting-bfi.ll

128

add CHECK-NOT

131

add a CHECK-NOT

include/llvm/Support/BlockFrequency.h

public:
 
   bool operator>(BlockFrequency RHS) const {
     return Frequency > RHS.Frequency;
   }
 
   bool operator>=(BlockFrequency RHS) const {
     return Frequency >= RHS.Frequency;
   }
+
+  bool operator==(BlockFrequency RHS) const {
+    return Frequency == RHS.Frequency;
+  }
 };
 
 }
 
 #endif

lib/Transforms/Scalar/ConstantHoisting.cpp

for (auto RIt = Orders.rbegin(); RIt != Orders.rend(); RIt++) {
     BasicBlock *Node = *RIt;
     bool NodeInBBs = BBs.count(Node);
     SmallPtrSet<BasicBlock *, 16> &InsertPts = InsertPtsMap[Node].first;
     BlockFrequency &InsertPtsFreq = InsertPtsMap[Node].second;
 
     // Return the optimal insert points in BBs.
     if (Node == Entry) {
       BBs.clear();
-      if (InsertPtsFreq > BFI.getBlockFreq(Node))
+      if (InsertPtsFreq > BFI.getBlockFreq(Node) ||
+          (InsertPtsFreq == BFI.getBlockFreq(Node) && InsertPts.size() > 1))
         BBs.insert(Entry);
       else
         BBs.insert(InsertPts.begin(), InsertPts.end());
       break;
     }
 
     BasicBlock *Parent = DT.getNode(Node)->getIDom()->getBlock();
     // Initially, ParentInsertPts is empty and ParentPtsFreq is 0. Every child
     // will update its parent's ParentInsertPts and ParentPtsFreq.
     SmallPtrSet<BasicBlock *, 16> &ParentInsertPts = InsertPtsMap[Parent].first;
     BlockFrequency &ParentPtsFreq = InsertPtsMap[Parent].second;
     // Choose to insert in Node or in subtree of Node.
-    if (InsertPtsFreq > BFI.getBlockFreq(Node) || NodeInBBs) {
+    // Don't hoist to EHPad because we may not find a proper place to insert
+    // in EHPad.
+    // If the total frequency of InsertPts is the same as the frequency of the
+    // target Node, and InsertPts contains more than one nodes, choose hoisting
+    // to reduce code size.
+    if (NodeInBBs ||
+        (!Node->isEHPad() &&
+         (InsertPtsFreq > BFI.getBlockFreq(Node) ||
+          (InsertPtsFreq == BFI.getBlockFreq(Node) && InsertPts.size() > 1)))) {
       ParentInsertPts.insert(Node);
       ParentPtsFreq += BFI.getBlockFreq(Node);
     } else {
       ParentInsertPts.insert(InsertPts.begin(), InsertPts.end());
       ParentPtsFreq += InsertPtsFreq;
     }
   }
 }

test/CodeGen/X86/constant-hoisting-bfi.ll

 ; RUN: opt -consthoist -mtriple=x86_64-unknown-linux-gnu -consthoist-with-block-frequency=true -S < %s | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
 ; Check when BFI is enabled for constant hoisting, constant 214748364701
 ; will not be hoisted to the func entry.
-; CHECK-LABEL: @foo(
+; CHECK-LABEL: @test1(
 ; CHECK: entry:
 ; CHECK-NOT: bitcast i64 214748364701 to i64
 ; CHECK: if.then:
 
 ; Function Attrs: norecurse nounwind uwtable
-define i64 @foo(i64* nocapture %a) {
+define i64 @test1(i64* nocapture %a) {
 entry:
   %arrayidx = getelementptr inbounds i64, i64* %a, i64 9
   %t0 = load i64, i64* %arrayidx, align 8
   %cmp = icmp slt i64 %t0, 564
   br i1 %cmp, label %if.then, label %if.else5
 
 if.then:                                          ; preds = %entry
   %arrayidx1 = getelementptr inbounds i64, i64* %a, i64 5
return:                                           ; preds = %if.else5, %if.then, %if.then8, %if.then3
   %retval.0 = phi i64 [ 214748364701, %if.then3 ], [ 214748364701, %if.then8 ], [ 250148364702, %if.then ], [ 256148364704, %if.else5 ]
   ret i64 %retval.0
 }
 
 ; Check when BFI is enabled for constant hoisting, constant 214748364701
 ; in while.body will be hoisted to while.body.preheader. 214748364701 in
 ; if.then16 and if.else10 will be merged and hoisted to the beginning of
 ; if.else10 because if.else10 dominates if.then16.
-; CHECK-LABEL: @goo(
+; CHECK-LABEL: @test2(
 ; CHECK: entry:
 ; CHECK-NOT: bitcast i64 214748364701 to i64
 ; CHECK: while.body.preheader:
 ; CHECK-NEXT: bitcast i64 214748364701 to i64
 ; CHECK-NOT: bitcast i64 214748364701 to i64
 ; CHECK: if.else10:
 ; CHECK-NEXT: bitcast i64 214748364701 to i64
 ; CHECK-NOT: bitcast i64 214748364701 to i64
-define i64 @goo(i64* nocapture %a) {
+define i64 @test2(i64* nocapture %a) {
 entry:
   %arrayidx = getelementptr inbounds i64, i64* %a, i64 9
   %t0 = load i64, i64* %arrayidx, align 8
   %cmp = icmp ult i64 %t0, 56
   br i1 %cmp, label %if.then, label %if.else10, !prof !0
 
 if.then:                                          ; preds = %entry
   %arrayidx1 = getelementptr inbounds i64, i64* %a, i64 5
while.cond.return.loopexit_crit_edge:             ; preds = %while.body
   br label %return
 
 return:                                           ; preds = %while.cond.preheader, %while.cond.return.loopexit_crit_edge, %if.else10, %if.then, %if.then16
   %retval.0 = phi i64 [ 214748364701, %if.then16 ], [ 0, %if.then ], [ 0, %if.else10 ], [ 0, %while.cond.return.loopexit_crit_edge ], [ 0, %while.cond.preheader ]
   ret i64 %retval.0
 }
 
 !0 = !{!"branch_weights", i32 1, i32 2000}
+
+; 214748364701 will be hoisted to entry block to reduce code size.
+; CHECK-LABEL: @test3(
+; CHECK: entry:
+; CHECK-NEXT: %const = bitcast i64 214748364701 to i64
+define i64 @test3(i64 %t0) {
+entry:
+  %cmp = icmp ult i64 %t0, 56
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+  %add1 = add i64 %t0, 214748364701
+  br label %return
+
+if.else:
+  %add2 = add i64 %t0, 214748364701
+  br label %return
+
+return:
+  %retval = phi i64 [ %add1, %if.then ], [ %add2, %if.else ]
+  ret i64 %retval
+}
+
+; 214748364701 will not be hoisted to entry block because it will only
+; increase its live range.
+; CHECK-LABEL: @test4(
+; CHECK: nextblock:
+; CHECK-NEXT: %add1 = add i64 %t0, 214748364701
+define i64 @test4(i64 %t0) {
+entry:
+  %cmp = icmp ult i64 %t0, 56
+  br label %nextblock
+
+nextblock:
+  %add1 = add i64 %t0, 214748364701
+  br label %return
+
+return:
+  ret i64 %add1
+}