[llvm] r343005 - Revert "[ConstHoist] Do not rebase single (or few) dependent constant"

Jessica Paquette via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 25 11:41:40 PDT 2018


Author: paquette
Date: Tue Sep 25 11:41:40 2018
New Revision: 343005

URL: http://llvm.org/viewvc/llvm-project?rev=343005&view=rev
Log:
Revert "[ConstHoist] Do not rebase single (or few) dependent constant"

This caused a couple test failures on a bot:

CodeGen/X86/constant-hoisting-bfi.ll
Transforms/ConstantHoisting/X86/ehpad.ll

Example:

http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/53575/

Removed:
    llvm/trunk/test/CodeGen/Thumb/consthoist-few-dependents.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp?rev=343005&r1=343004&r2=343005&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp Tue Sep 25 11:41:40 2018
@@ -86,12 +86,6 @@ static cl::opt<bool> ConstHoistGEP(
     "consthoist-gep", cl::init(false), cl::Hidden,
     cl::desc("Try hoisting constant gep expressions"));
 
-static cl::opt<unsigned>
-MinNumOfDependentToRebase("consthoist-min-num-to-rebase",
-    cl::desc("Do not rebase if number of dependent constants of a Base is less "
-             "than this number."),
-    cl::init(2), cl::Hidden);
-
 namespace {
 
 /// The constant hoisting pass.
@@ -828,34 +822,7 @@ bool ConstantHoistingPass::emitBaseConst
 
     unsigned UsesNum = 0;
     unsigned ReBasesNum = 0;
-    unsigned NotRebasedNum = 0;
     for (Instruction *IP : IPSet) {
-      // First, collect constants depending on this IP of the base.
-      unsigned Uses = 0;
-      using RebasedUse = std::tuple<Constant *, Type *, ConstantUser>;
-      SmallVector<RebasedUse, 4> ToBeRebased;
-      for (auto const &RCI : ConstInfo.RebasedConstants) {
-        for (auto const &U : RCI.Uses) {
-          Uses++;
-          BasicBlock *OrigMatInsertBB =
-              findMatInsertPt(U.Inst, U.OpndIdx)->getParent();
-          // If Base constant is to be inserted in multiple places,
-          // generate rebase for U using the Base dominating U.
-          if (IPSet.size() == 1 ||
-              DT->dominates(IP->getParent(), OrigMatInsertBB))
-            ToBeRebased.push_back(RebasedUse(RCI.Offset, RCI.Ty, U));
-        }
-      }
-      UsesNum = Uses;
-
-      // If only few constants depend on this IP of base, skip rebasing,
-      // assuming the base and the rebased have the same materialization cost.
-      if (ToBeRebased.size() < MinNumOfDependentToRebase) {
-        NotRebasedNum += ToBeRebased.size();
-        continue;
-      }
-
-      // Emit an instance of the base at this IP.
       Instruction *Base = nullptr;
       // Hoist and hide the base constant behind a bitcast.
       if (ConstInfo.BaseExpr) {
@@ -873,27 +840,36 @@ bool ConstantHoistingPass::emitBaseConst
                         << ") to BB " << IP->getParent()->getName() << '\n'
                         << *Base << '\n');
 
-      // Emit materialization code for rebased constants depending on this IP.
-      for (auto const &R : ToBeRebased) {
-        Constant *Off = std::get<0>(R);
-        Type *Ty = std::get<1>(R);
-        ConstantUser U = std::get<2>(R);
-        emitBaseConstants(Base, Off, Ty, U);
-        ReBasesNum++;
-        // Use the same debug location as the last user of the constant.
-        Base->setDebugLoc(DILocation::getMergedLocation(
-            Base->getDebugLoc(), U.Inst->getDebugLoc()));
+      // Emit materialization code for all rebased constants.
+      unsigned Uses = 0;
+      for (auto const &RCI : ConstInfo.RebasedConstants) {
+        for (auto const &U : RCI.Uses) {
+          Uses++;
+          BasicBlock *OrigMatInsertBB =
+              findMatInsertPt(U.Inst, U.OpndIdx)->getParent();
+          // If Base constant is to be inserted in multiple places,
+          // generate rebase for U using the Base dominating U.
+          if (IPSet.size() == 1 ||
+              DT->dominates(Base->getParent(), OrigMatInsertBB)) {
+            emitBaseConstants(Base, RCI.Offset, RCI.Ty, U);
+            ReBasesNum++;
+          }
+
+          Base->setDebugLoc(DILocation::getMergedLocation(
+              Base->getDebugLoc(), U.Inst->getDebugLoc()));
+        }
       }
+      UsesNum = Uses;
+
+      // Use the same debug location as the last user of the constant.
       assert(!Base->use_empty() && "The use list is empty!?");
       assert(isa<Instruction>(Base->user_back()) &&
              "All uses should be instructions.");
     }
     (void)UsesNum;
     (void)ReBasesNum;
-    (void)NotRebasedNum;
     // Expect all uses are rebased after rebase is done.
-    assert(UsesNum == (ReBasesNum + NotRebasedNum) &&
-           "Not all uses are rebased");
+    assert(UsesNum == ReBasesNum && "Not all uses are rebased");
 
     NumConstantsHoisted++;
 

Removed: llvm/trunk/test/CodeGen/Thumb/consthoist-few-dependents.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Thumb/consthoist-few-dependents.ll?rev=343004&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/Thumb/consthoist-few-dependents.ll (original)
+++ llvm/trunk/test/CodeGen/Thumb/consthoist-few-dependents.ll (removed)
@@ -1,191 +0,0 @@
-; RUN: opt -consthoist -S %s -o - | FileCheck %s --check-prefix=OPT
-; RUN: opt -consthoist -S -consthoist-min-num-to-rebase=1 %s -o - | FileCheck %s --check-prefix=OPT --check-prefix=OPT-1
-; RUN: opt -consthoist -S -consthoist-min-num-to-rebase=2 %s -o - | FileCheck %s --check-prefix=OPT --check-prefix=OPT-2
-; RUN: opt -consthoist -S -consthoist-min-num-to-rebase=3 %s -o - | FileCheck %s --check-prefix=OPT --check-prefix=OPT-3
-
-; RUN: llc %s -o - | FileCheck %s --check-prefix=LLC
-
-target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
-target triple = "thumbv6m-none-unknown-musleabi"
-
-; Test that constant 0 and 1 of i1 type is NOT hoisted due low
-; materializing cost.
-
-; OPT-LABEL: avalon
-; OPT: bb1:
-; OPT: store i1 true
-; OPT: bb2:
-; OPT: store i1 false
-; OPT: bb3:
-; OPT: store i1 false
-; OPT: store i1 false
-; OPT-NOT: add
-
-; LLC-LABEL: avalon
-; LLC-DAG: movs r{{[0-9]+}}, #0
-; LLC-DAG: movs r{{[0-9]+}}, #0
-; LLC-DAG: movs r{{[0-9]+}}, #1
-; LLC-NOT: add
-
- at global = local_unnamed_addr global i1 undef, align 1
- at global.0 = local_unnamed_addr global i1 undef, align 1
-
-define void @avalon() #0 {
-bb:
-  switch i8 undef, label %bb5 [
-    i8 0, label %bb1
-    i8 -1, label %bb2
-    i8 1, label %bb3
-  ]
-
-bb1:
-  store i1 1, i1* @global, align 1
-  unreachable
-
-bb2:
-  store i1 0, i1* @global, align 1
-  unreachable
-
-bb3:
-  store i1 0, i1* @global.0, align 1
-  store i1 0, i1* @global, align 1
-  unreachable
-
-bb5:
-  ret void
-}
-
-; Test that for i8 type, constant -1 is not rebased since it's the only
-; dependent of base constant -2.
-; This test is also covered by r342898, see
-; test/CodeGen/Thumb/consthoist-imm8-costs-1.ll
-
-; OPT-LABEL: barney
-; OPT: bb1:
-; OPT: store i8 -1
-; OPT: bb2:
-; OPT: store i8 -2
-; OPT: bb3:
-; OPT: store i8 -2
-; OPT: store i8 -2
-; OPT-NOT: add
-
-; LLC-LABEL: barney
-; LLC-DAG: movs r{{[0-9]+}}, #254
-; LLC-DAG: movs r{{[0-9]+}}, #255
-; LLC-NOT: mvn
-; LLC-NOT: add
-
- at global.1 = local_unnamed_addr global i8 undef, align 1
- at global.2 = local_unnamed_addr global i8 undef, align 1
-
-define void @barney() #0 {
-bb:
-  switch i8 undef, label %bb5 [
-    i8 0, label %bb1
-    i8 -1, label %bb2
-    i8 1, label %bb3
-  ]
-
-bb1:                                              ; preds = %bb
-  store i8 -1, i8* @global.1, align 1
-  unreachable
-
-bb2:                                              ; preds = %bb
-  store i8 -2, i8* @global.1, align 1
-  unreachable
-
-bb3:                                              ; preds = %bb
-  store i8 -2, i8* @global.2, align 1
-  store i8 -2, i8* @global.1, align 1
-  unreachable
-
-bb5:                                              ; preds = %bb
-  ret void
-}
-
-; Test that for i16 type constant 65532 is not rebased if it's the only
-; dependent of base constant 65531. Cost would be the same if rebased.
-; If rebased, 3 two-byte instructions:
-;   movs    r0, #4
-;   mvns    r0, r0
-;   adds    r0, r0, #1
-; If NOT rebased, 1 two-byte instruction plus 1 four-byte CP entry:
-;           ldr     r1, .LCPI2_3
-;           ...
-;   .LCPI2_3:
-;           .long   65532
-
-; OPT-LABEL: carla
-
-; -consthoist-min-num-to-rebase=1, check that 65532 and single use of 65531
-; in bb2 is rebased
-; OPT-1: bb1:
-; OPT-1: %[[C1:const[0-9]?]] = bitcast i16 -5 to i16
-; OPT-1-NEXT: %const_mat = add i16 %[[C1]], 1
-; OPT-1-NEXT: store i16 %const_mat, i16* @global.3, align 1
-; OPT-1: bb2:
-; OPT-1-NEXT: %[[C2:const[0-9]?]] = bitcast i16 -5 to i16
-; OPT-1-NEXT: store i16 %[[C2]], i16* @global.3, align 1
-; OPT-1: bb3:
-; OPT-1-NEXT: %[[C3:const[0-9]?]] = bitcast i16 -5 to i16
-; OPT-1-NEXT: store i16 %[[C3]], i16* @global.4, align 1
-; OPT-1-NEXT: store i16 %[[C3]], i16* @global.3, align 1
-
-; -consthoist-min-num-to-rebase=2, check that 65532 and single use of 65531
-; in bb2 is not rebased
-; OPT-2: bb1:
-; OPT-2-NEXT: store i16 -4, i16* @global.3, align 1
-; OPT-2: bb2:
-; OPT-2-NEXT: store i16 -5, i16* @global.3, align 1
-; OPT-2: bb3:
-; OPT-2-NEXT:   %[[C4:const[0-9]?]] = bitcast i16 -5 to i16
-; OPT-2-NEXT:   store i16 %[[C4]], i16* @global.4, align 1
-; OPT-2-NEXT:   store i16 %[[C4]], i16* @global.3, align 1
-; OPT-2-NOT: add
-
-; -consthoist-min-num-to-rebase=3, check that dual uses of 65531 in bb3 are
-; not rebase
-; OPT-3: bb1:
-; OPT-3-NEXT: store i16 -4, i16* @global.3, align 1
-; OPT-3: bb2:
-; OPT-3-NEXT: store i16 -5, i16* @global.3, align 1
-; OPT-3: bb3:
-; OPT-3-NEXT:   store i16 -5, i16* @global.4, align 1
-; OPT-3-NEXT:   store i16 -5, i16* @global.3, align 1
-; OPT-3-NOT: add
-; OPT-3-NOT: bitcast
-
-; LLC-LABEL: carla
-; LLC-DAG: ldr r{{[0-9]+}}, .LCPI2_1
-; LLC-DAG: ldr r{{[0-9]+}}, .LCPI2_3
-; LLC-NOT: mvn
-; LLC-NOT: add
-
- at global.3 = local_unnamed_addr global i16 undef, align 2
- at global.4 = local_unnamed_addr global i16 undef, align 2
-
-define void @carla() {
-bb:
-  switch i8 undef, label %bb5 [
-    i8 0, label %bb1
-    i8 -1, label %bb2
-    i8 1, label %bb3
-  ]
-
-bb1:                                              ; preds = %bb
-  store i16 65532, i16* @global.3, align 1
-  unreachable
-
-bb2:                                              ; preds = %bb
-  store i16 65531, i16* @global.3, align 1
-  unreachable
-
-bb3:                                              ; preds = %bb
-  store i16 65531, i16* @global.4, align 1
-  store i16 65531, i16* @global.3, align 1
-  unreachable
-
-bb5:                                              ; preds = %bb
-  ret void
-}




More information about the llvm-commits mailing list