[ConstHoisting] Turn on consthoist-with-block-frequency by default

[wmi]

Summary

Using profile information to guide consthoisting is generally helpful for performance, so we want to turn it on by default. No compile time or perf regression were found using spec2000 and spec2006 on x86. Some significant improvement (>20%) was seen on internal benchmarks.

[davidxl]

test/CodeGen/X86/fold-tied-op.ll

10 ↗

(On Diff #105450)

explain this change?

test/Transforms/ConstantHoisting/X86/ehpad.ll

4 ↗

(On Diff #105450)

Why removing the comment?

[wmi]

test/CodeGen/X86/fold-tied-op.ll

10 ↗

(On Diff #105450)

After BFI based const hoisting enabled, some const is not hoisted to func entry. This will change live range and affect scheduling for live range shrinking (https://reviews.llvm.org/D32563).

test/Transforms/ConstantHoisting/X86/ehpad.ll

4 ↗

(On Diff #105450)

The bitcast is used for const 9209618997431186100. Now the bitcast is inserted in catchpad block when consthoist-with-block-frequency is enabled so the FIXME can be removed.

[davidxl]

test/CodeGen/X86/fold-tied-op.ll

10 ↗

(On Diff #105450)

Can you add profile annotation so that the test behavior does not change?

[wmi]

test/CodeGen/X86/fold-tied-op.ll

10 ↗

(On Diff #105450)

Adding profile annotation doesn't change the result. Currently BFI based const hoisting only hoists the const when the freq of the target bb is colder, so for case like "if(cond) then use constA; else use constA", although constA is used both in then and else branch, it will not be hoisted before if(cond) because the freq doesn't change after the hoisting.

[davidxl]

test/CodeGen/X86/fold-tied-op.ll

10 ↗

(On Diff #105450)

If the frequency is equal, should it be hoisted (as it reduces size at least)?

[wmi]

test/CodeGen/X86/fold-tied-op.ll

10 ↗

(On Diff #105450)

Yes, I can do that in a separate patch.

[davidxl]

lgtm

llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp

 using namespace consthoist;
 
 #define DEBUG_TYPE "consthoist"
 
 STATISTIC(NumConstantsHoisted, "Number of constants hoisted");
 STATISTIC(NumConstantsRebased, "Number of constants rebased");
 
 static cl::opt<bool> ConstHoistWithBlockFrequency(
-    "consthoist-with-block-frequency", cl::init(false), cl::Hidden,
+    "consthoist-with-block-frequency", cl::init(true), cl::Hidden,
     cl::desc("Enable the use of the block frequency analysis to reduce the "
              "chance to execute const materialization more frequently than "
              "without hoisting."));
 
 namespace {
 /// \brief The constant hoisting pass.
 class ConstantHoistingLegacyPass : public FunctionPass {
 public:

llvm/trunk/test/Transforms/ConstantHoisting/X86/ehpad.ll

-; RUN: opt -S -consthoist < %s | FileCheck %s
+; RUN: opt -S -consthoist -consthoist-with-block-frequency=false < %s | FileCheck %s
 ; RUN: opt -S -consthoist -consthoist-with-block-frequency=true < %s | FileCheck --check-prefix=BFIHOIST %s
 
-; FIXME: The catchpad doesn't even use the constant, so a better fix would be to
-; insert the bitcast in the catchpad block.
-
 target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-windows-msvc"
 
 ; CHECK-LABEL: define i32 @main
 ; CHECK: %tobool = icmp eq i32 %argc, 0
 ; CHECK-NEXT: bitcast i64 9209618997431186100 to i64
 ; CHECK-NEXT: br i1 %tobool