[NFCI][SimplifyCFG] Guard common code hoisting with a (default-on) flag

Common code sinking is already guarded with a (with default-off!) flag, so add a flag for hoisting, too. D84108 will hopefully make hoisting off-by-default too.
llvm · Jul 20, 2020 · 04b729d · 04b729d
1 parent 3de4166
commit 04b729d
Show file tree

Hide file tree

Showing 24 changed files with 58 additions and 22 deletions.
diff --git a/llvm/include/llvm/Transforms/Utils/SimplifyCFGOptions.h b/llvm/include/llvm/Transforms/Utils/SimplifyCFGOptions.h
@@ -25,6 +25,7 @@ struct SimplifyCFGOptions {
   bool ForwardSwitchCondToPhi = false;
   bool ConvertSwitchToLookupTable = false;
   bool NeedCanonicalLoop = true;
+  bool HoistCommonInsts = true;
   bool SinkCommonInsts = false;
   bool SimplifyCondBranch = true;
   bool FoldTwoEntryPHINode = true;
@@ -48,6 +49,10 @@ struct SimplifyCFGOptions {
     NeedCanonicalLoop = B;
     return *this;
   }
+  SimplifyCFGOptions &hoistCommonInsts(bool B) {
+    HoistCommonInsts = B;
+    return *this;
+  }
   SimplifyCFGOptions &sinkCommonInsts(bool B) {
     SinkCommonInsts = B;
     return *this;

diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
@@ -1763,6 +1763,8 @@ Expected<SimplifyCFGOptions> parseSimplifyCFGOptions(StringRef Params) {
       Result.convertSwitchToLookupTable(Enable);
     } else if (ParamName == "keep-loops") {
       Result.needCanonicalLoops(Enable);
+    } else if (ParamName == "hoist-common-insts") {
+      Result.hoistCommonInsts(Enable);
     } else if (ParamName == "sink-common-insts") {
       Result.sinkCommonInsts(Enable);
     } else if (Enable && ParamName.consume_front("bonus-inst-threshold=")) {

diff --git a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
@@ -62,6 +62,10 @@ static cl::opt<bool> UserForwardSwitchCond(
     "forward-switch-cond", cl::Hidden, cl::init(false),
     cl::desc("Forward switch condition to phi ops (default = false)"));
 
+static cl::opt<bool> UserHoistCommonInsts(
+    "hoist-common-insts", cl::Hidden, cl::init(true),
+    cl::desc("hoist common instructions (default = true)"));
+
 static cl::opt<bool> UserSinkCommonInsts(
     "sink-common-insts", cl::Hidden, cl::init(false),
     cl::desc("Sink common instructions (default = false)"));
@@ -222,6 +226,8 @@ static void applyCommandLineOverridesToOptions(SimplifyCFGOptions &Options) {
     Options.ConvertSwitchToLookupTable = UserSwitchToLookup;
   if (UserKeepLoops.getNumOccurrences())
     Options.NeedCanonicalLoop = UserKeepLoops;
+  if (UserHoistCommonInsts.getNumOccurrences())
+    Options.HoistCommonInsts = UserHoistCommonInsts;
   if (UserSinkCommonInsts.getNumOccurrences())
     Options.SinkCommonInsts = UserSinkCommonInsts;
 }

diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -105,6 +105,10 @@ static cl::opt<bool> DupRet(
     "simplifycfg-dup-ret", cl::Hidden, cl::init(false),
     cl::desc("Duplicate return instructions into unconditional branches"));
 
+static cl::opt<bool>
+    HoistCommon("simplifycfg-hoist-common", cl::Hidden, cl::init(true),
+                cl::desc("Hoist common instructions up to the parent block"));
+
 static cl::opt<bool>
     SinkCommon("simplifycfg-sink-common", cl::Hidden, cl::init(true),
                cl::desc("Sink common instructions down to the end block"));
@@ -6063,8 +6067,9 @@ bool SimplifyCFGOpt::simplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) {
   // can hoist it up to the branching block.
   if (BI->getSuccessor(0)->getSinglePredecessor()) {
     if (BI->getSuccessor(1)->getSinglePredecessor()) {
-      if (HoistThenElseCodeToIf(BI, TTI))
-        return requestResimplify();
+      if (HoistCommon && Options.HoistCommonInsts)
+        if (HoistThenElseCodeToIf(BI, TTI))
+          return requestResimplify();
     } else {
       // If Successor #1 has multiple preds, we may be able to conditionally
       // execute Successor #0 if it branches to Successor #1.

diff --git a/llvm/test/Transforms/GVNSink/indirect-call.ll b/llvm/test/Transforms/GVNSink/indirect-call.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -gvn-sink -simplifycfg -simplifycfg-sink-common=false -S | FileCheck %s
+; RUN: opt < %s -gvn-sink -simplifycfg -hoist-common-insts=true -simplifycfg-sink-common=false -S | FileCheck %s
 
 declare i8 @ext(i1)
 

diff --git a/llvm/test/Transforms/GVNSink/sink-common-code.ll b/llvm/test/Transforms/GVNSink/sink-common-code.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -gvn-sink -simplifycfg -simplifycfg-sink-common=false -S | FileCheck %s
+; RUN: opt < %s -gvn-sink -simplifycfg -hoist-common-insts=true -simplifycfg-sink-common=false -S | FileCheck %s
 
 define zeroext i1 @test1(i1 zeroext %flag, i32 %blksA, i32 %blksB, i32 %nblks) {
 entry:

diff --git a/llvm/test/Transforms/SimplifyCFG/2008-12-16-DCECond.ll b/llvm/test/Transforms/SimplifyCFG/2008-12-16-DCECond.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -simplifycfg -S | not grep icmp
+; RUN: opt < %s -simplifycfg -S -hoist-common-insts=true | not grep icmp
 ; ModuleID = '/tmp/x.bc'
 target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128"
 target triple = "i686-pc-linux-gnu"

diff --git a/llvm/test/Transforms/SimplifyCFG/AArch64/prefer-fma.ll b/llvm/test/Transforms/SimplifyCFG/AArch64/prefer-fma.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -mtriple=aarch64-linux-gnu -simplifycfg -enable-unsafe-fp-math -S >%t
+; RUN: opt < %s -mtriple=aarch64-linux-gnu -simplifycfg -hoist-common-insts=true -enable-unsafe-fp-math -S >%t
 ; RUN: FileCheck %s < %t
 ; ModuleID = 't.cc'
 

diff --git a/llvm/test/Transforms/SimplifyCFG/BrUnwind.ll b/llvm/test/Transforms/SimplifyCFG/BrUnwind.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -simplifycfg -S | \
+; RUN: opt < %s -simplifycfg -hoist-common-insts=true -S | \
 ; RUN: not grep "br label"
 
 define void @test(i1 %C) {

diff --git a/llvm/test/Transforms/SimplifyCFG/HoistCode.ll b/llvm/test/Transforms/SimplifyCFG/HoistCode.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -simplifycfg -S | FileCheck %s
+; RUN: opt < %s -simplifycfg -hoist-common-insts=true -S | FileCheck %s
 
 define void @foo(i1 %C, i32* %P) {
 ; CHECK-LABEL: @foo(

diff --git a/llvm/test/Transforms/SimplifyCFG/PowerPC/prefer-fma.ll b/llvm/test/Transforms/SimplifyCFG/PowerPC/prefer-fma.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -mtriple=powerpc64le-unknown-linux-gnu -simplifycfg -enable-unsafe-fp-math -S | \
+; RUN: opt < %s -mtriple=powerpc64le-unknown-linux-gnu -simplifycfg -hoist-common-insts=true -enable-unsafe-fp-math -S | \
 ; RUN: FileCheck %s
 
 ; This case is copied from test/Transforms/SimplifyCFG/AArch64/

diff --git a/llvm/test/Transforms/SimplifyCFG/PowerPC/prefer-load-i32.ll b/llvm/test/Transforms/SimplifyCFG/PowerPC/prefer-load-i32.ll
@@ -1,5 +1,4 @@
-; RUN: opt < %s -mtriple=powerpc64le-unknown-linux-gnu -simplifycfg -S | \
-; RUN: FileCheck %s
+; RUN: opt < %s -mtriple=powerpc64le-unknown-linux-gnu -simplifycfg -hoist-common-insts=true -S | FileCheck %s
 
 define float @foo(float* %src, float* %dest, i32 signext %count, i32 signext %cond) {
 ; CHECK-LABEL: @foo(

diff --git a/llvm/test/Transforms/SimplifyCFG/UncondBranchToReturn.ll b/llvm/test/Transforms/SimplifyCFG/UncondBranchToReturn.ll
@@ -2,7 +2,7 @@
 ; a PHI node and a return.  Make sure the simplify cfg can straighten out this
 ; important case.  This is basically the most trivial form of tail-duplication.
 
-; RUN: opt < %s -simplifycfg -S | \
+; RUN: opt < %s -simplifycfg -hoist-common-insts=true -S | \
 ; RUN:    not grep "br label"
 
 define i32 @test(i1 %B, i32 %A, i32 %B.upgrd.1) {

diff --git a/llvm/test/Transforms/SimplifyCFG/X86/empty-cleanuppad.ll b/llvm/test/Transforms/SimplifyCFG/X86/empty-cleanuppad.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -simplifycfg -S | FileCheck %s
+; RUN: opt < %s -simplifycfg -S -hoist-common-insts=true | FileCheck %s
 
 ; ModuleID = 'cppeh-simplify.cpp'
 target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"

diff --git a/llvm/test/Transforms/SimplifyCFG/X86/pr39187-g.ll b/llvm/test/Transforms/SimplifyCFG/X86/pr39187-g.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -S -simplifycfg | FileCheck %s
+; RUN: opt < %s -S -simplifycfg -hoist-common-insts=true | FileCheck %s
 
 ; SimplifyCFG can hoist any common code in the 'then' and 'else' blocks to
 ; the 'if' basic block.

diff --git a/llvm/test/Transforms/SimplifyCFG/X86/remove-debug.ll b/llvm/test/Transforms/SimplifyCFG/X86/remove-debug.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -simplifycfg -S | FileCheck %s
+; RUN: opt < %s -simplifycfg -S -hoist-common-insts=true | FileCheck %s
 
 ; TODO: Track the acutal DebugLoc of the hoisted instruction when no-line
 ; DebugLoc is supported (https://reviews.llvm.org/D24180)

diff --git a/llvm/test/Transforms/SimplifyCFG/common-code-hoisting.ll b/llvm/test/Transforms/SimplifyCFG/common-code-hoisting.ll
@@ -1,4 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -simplifycfg -hoist-common-insts=1 -S < %s                    | FileCheck %s --check-prefixes=HOIST
+; RUN: opt -simplifycfg -hoist-common-insts=0 -S < %s                    | FileCheck %s --check-prefixes=NOHOIST
 ; RUN: opt -simplifycfg                       -S < %s                    | FileCheck %s --check-prefixes=HOIST,DEFAULT
 
 ; This example is produced from a very basic C code:
@@ -57,6 +59,23 @@ define void @_Z4loopi(i1 %cmp) {
 ; HOIST:       return:
 ; HOIST-NEXT:    ret void
 ;
+; NOHOIST-LABEL: @_Z4loopi(
+; NOHOIST-NEXT:  entry:
+; NOHOIST-NEXT:    br i1 [[CMP:%.*]], label [[RETURN:%.*]], label [[FOR_COND:%.*]]
+; NOHOIST:       for.cond:
+; NOHOIST-NEXT:    [[CMP1:%.*]] = call i1 @gen1()
+; NOHOIST-NEXT:    br i1 [[CMP1]], label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
+; NOHOIST:       for.body:
+; NOHOIST-NEXT:    call void @f0()
+; NOHOIST-NEXT:    call void @f1()
+; NOHOIST-NEXT:    br label [[FOR_COND]]
+; NOHOIST:       for.end:
+; NOHOIST-NEXT:    call void @f0()
+; NOHOIST-NEXT:    call void @f2()
+; NOHOIST-NEXT:    br label [[RETURN]]
+; NOHOIST:       return:
+; NOHOIST-NEXT:    ret void
+;
 entry:
   br i1 %cmp, label %if.then, label %if.end
 

diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-common-code.ll b/llvm/test/Transforms/SimplifyCFG/hoist-common-code.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -simplifycfg -S | not grep br
+; RUN: opt < %s -simplifycfg -S -hoist-common-insts=true | not grep br
 
 declare void @bar(i32)
 

diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll b/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll
@@ -1,4 +1,4 @@
-; RUN: opt -simplifycfg -S < %s | FileCheck %s
+; RUN: opt -simplifycfg -hoist-common-insts=true -S < %s | FileCheck %s
 ; Verify that we don't crash due an invalid !dbg location on the hoisted llvm.dbg.value
 
 define i64 @caller(i64* %ptr, i64 %flag) !dbg !10 {

diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-with-range.ll b/llvm/test/Transforms/SimplifyCFG/hoist-with-range.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -simplifycfg -S | FileCheck %s
+; RUN: opt < %s -simplifycfg -hoist-common-insts=true -S | FileCheck %s
 
 define void @foo(i1 %c, i8* %p) {
 ; CHECK: if:

diff --git a/llvm/test/Transforms/SimplifyCFG/pr39807.ll b/llvm/test/Transforms/SimplifyCFG/pr39807.ll
@@ -1,4 +1,4 @@
-; RUN: opt -S -simplifycfg < %s | FileCheck %s
+; RUN: opt -S -simplifycfg -hoist-common-insts=true < %s | FileCheck %s
 
 declare void @personality()
 

diff --git a/llvm/test/Transforms/SimplifyCFG/preserve-load-metadata-2.ll b/llvm/test/Transforms/SimplifyCFG/preserve-load-metadata-2.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -simplifycfg -S | FileCheck %s
+; RUN: opt < %s -simplifycfg -hoist-common-insts=true -S | FileCheck %s
 
 declare void @bar(i32*)
 declare void @baz(i32*)

diff --git a/llvm/test/Transforms/SimplifyCFG/preserve-load-metadata-3.ll b/llvm/test/Transforms/SimplifyCFG/preserve-load-metadata-3.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -simplifycfg -S | FileCheck %s
+; RUN: opt < %s -simplifycfg -hoist-common-insts=true -S | FileCheck %s
 
 declare void @bar(i32*)
 declare void @baz(i32*)

diff --git a/llvm/test/Transforms/SimplifyCFG/preserve-load-metadata.ll b/llvm/test/Transforms/SimplifyCFG/preserve-load-metadata.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -simplifycfg -S | FileCheck %s
+; RUN: opt < %s -simplifycfg -hoist-common-insts=true -S | FileCheck %s
 
 declare void @bar(i32*)
 declare void @baz(i32*)