Index: lib/Transforms/Scalar/ConstantHoisting.cpp =================================================================== --- lib/Transforms/Scalar/ConstantHoisting.cpp +++ lib/Transforms/Scalar/ConstantHoisting.cpp @@ -86,6 +86,12 @@ "consthoist-gep", cl::init(false), cl::Hidden, cl::desc("Try hoisting constant gep expressions")); +static cl::opt +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. @@ -822,7 +828,34 @@ 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; + SmallVector 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) { @@ -840,36 +873,27 @@ << ") to BB " << IP->getParent()->getName() << '\n' << *Base << '\n'); - // 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())); - } + // 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())); } - 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(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 && "Not all uses are rebased"); + assert(UsesNum == (ReBasesNum + NotRebasedNum) && + "Not all uses are rebased"); NumConstantsHoisted++; Index: test/CodeGen/Thumb/consthoist-single-dependent.ll =================================================================== --- /dev/null +++ test/CodeGen/Thumb/consthoist-single-dependent.ll @@ -0,0 +1,190 @@ +; 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 + +@global = local_unnamed_addr global i1 undef, align 1 +@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 https://reviews.llvm.org/D52257. + +; 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 + +@global.1 = local_unnamed_addr global i8 undef, align 1 +@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 + +@global.3 = local_unnamed_addr global i16 undef, align 2 +@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 +}