[llvm-dev] [llvm] r342387 - [GVNHoist] Re-enable GVNHoist by default

Tue Oct 2 16:56:14 PDT 2018

OK. Thanks!

-eric

On Tue, Oct 2, 2018 at 2:16 AM Alexandros Lamprineas <
Alexandros.Lamprineas at arm.com> wrote:

> Hi Eric,
>
>
> Thanks for your feedback. I am not actively working on GVN-Hoist, but I'll
> take a look at these benchmarks at some point. We can keep it reverted for
> the time being. My take is that the community should get more involved if
> GVN-Hoist is an optimization we would like to re-enable. I felt I was a bit
> on my own trying to get it to a good state.
>
>
> Regards,
>
> Alexandros
> ------------------------------
> *From:* Eric Christopher <echristo at gmail.com>
> *Sent:* Monday, October 1, 2018 7:59:21 PM
> *To:* Alexandros Lamprineas
> *Cc:* llvm-commits at lists.llvm.org; Jordan Rupprecht; Jorge Gorbe Moya
> *Subject:* Re: [llvm] r342387 - [GVNHoist] Re-enable GVNHoist by default
>
> And reverted here:
>
> echristo at athyra ~/s/llvm> git svn dcommit
> Committing to https://llvm.org/svn/llvm-project/llvm/trunk ...
> M lib/Passes/PassBuilder.cpp
> M lib/Transforms/IPO/PassManagerBuilder.cpp
> M test/Other/new-pm-defaults.ll
> M test/Other/new-pm-thinlto-defaults.ll
> M test/Other/opt-O2-pipeline.ll
> M test/Other/opt-O3-pipeline.ll
> M test/Other/opt-Os-pipeline.ll
> Committed r343522
>
> Thanks!
>
> On Mon, Oct 1, 2018 at 11:48 AM Eric Christopher <echristo at gmail.com>
> wrote:
>
> As a follow-up the benchmarks are now available here:
>
> https://github.com/google/hashtable-benchmarks
>
> I'm going to go ahead and revert this in the meantime. I haven't seen
> performance numbers with GVNHoist enabled - could you make sure that those
> are also in the next commit message for review?
>
> Thanks!
>
> On Fri, Sep 28, 2018 at 11:27 AM Eric Christopher <echristo at gmail.com>
> wrote:
>
> HI Alexandros
>
> We're seeing multiple large regressions (~25%) in performance that are
> pointing at re-enabling gvn hoist again. A notable piece of visible code is
> swisstable, which can be found here:
> https://github.com/abseil/abseil-cpp/tree/master/absl/container. The
> benchmarks aren't currently there and I'll work on getting you some, but
> you might be able to take a look there.
>
> I've got other examples too and this would be widespread enough that I'd
> want to disable again until we can get the performance back or at least
> closer, overall performance greater with no large regressions is the goal
> right?
>
> Thanks!
>
> -eric
>
> On Mon, Sep 17, 2018 at 5:26 AM Alexandros Lamprineas via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
> Author: alelab01
> Date: Mon Sep 17 05:24:55 2018
> New Revision: 342387
>
> URL: http://llvm.org/viewvc/llvm-project?rev=342387&view=rev
> Log:
> [GVNHoist] Re-enable GVNHoist by default
>
> Rebase rL341954 since https://bugs.llvm.org/show_bug.cgi?id=38912
> has been fixed by rL342055.
>
> Precommit testing performed:
> * Overnight runs of csmith comparing the output between programs
>   compiled with gvn-hoist enabled/disabled.
> * Bootstrap builds of clang with UbSan/ASan configurations.
>
> Modified:
>     llvm/trunk/lib/Passes/PassBuilder.cpp
>     llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp
>     llvm/trunk/test/Other/new-pm-defaults.ll
>     llvm/trunk/test/Other/new-pm-thinlto-defaults.ll
>     llvm/trunk/test/Other/opt-O2-pipeline.ll
>     llvm/trunk/test/Other/opt-O3-pipeline.ll
>     llvm/trunk/test/Other/opt-Os-pipeline.ll
>
> Modified: llvm/trunk/lib/Passes/PassBuilder.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Passes/PassBuilder.cpp?rev=342387&r1=342386&r2=342387&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Passes/PassBuilder.cpp (original)
> +++ llvm/trunk/lib/Passes/PassBuilder.cpp Mon Sep 17 05:24:55 2018
> @@ -175,8 +175,8 @@ static cl::opt<bool> EnableEarlyCSEMemSS
>      cl::desc("Enable the EarlyCSE w/ MemorySSA pass for the new PM
> (default = on)"));
>
>  static cl::opt<bool> EnableGVNHoist(
> -    "enable-npm-gvn-hoist", cl::init(false), cl::Hidden,
> -    cl::desc("Enable the GVN hoisting pass for the new PM (default =
> off)"));
> +    "enable-npm-gvn-hoist", cl::init(true), cl::Hidden,
> +    cl::desc("Enable the GVN hoisting pass for the new PM (default =
> on)"));
>
>  static cl::opt<bool> EnableGVNSink(
>      "enable-npm-gvn-sink", cl::init(false), cl::Hidden,
>
> Modified: llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp?rev=342387&r1=342386&r2=342387&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp (original)
> +++ llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp Mon Sep 17
> 05:24:55 2018
> @@ -139,8 +139,8 @@ static cl::opt<bool> EnableEarlyCSEMemSS
>      cl::desc("Enable the EarlyCSE w/ MemorySSA pass (default = on)"));
>
>  static cl::opt<bool> EnableGVNHoist(
> -    "enable-gvn-hoist", cl::init(false), cl::Hidden,
> -    cl::desc("Enable the GVN hoisting pass (default = off)"));
> +    "enable-gvn-hoist", cl::init(true), cl::Hidden,
> +    cl::desc("Enable the GVN hoisting pass (default = on)"));
>
>  static cl::opt<bool>
>      DisableLibCallsShrinkWrap("disable-libcalls-shrinkwrap",
> cl::init(false),
>
> Modified: llvm/trunk/test/Other/new-pm-defaults.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/new-pm-defaults.ll?rev=342387&r1=342386&r2=342387&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Other/new-pm-defaults.ll (original)
> +++ llvm/trunk/test/Other/new-pm-defaults.ll Mon Sep 17 05:24:55 2018
> @@ -121,6 +121,10 @@
>  ; CHECK-O-NEXT: Running pass: SROA
>  ; CHECK-O-NEXT: Running pass: EarlyCSEPass
>  ; CHECK-O-NEXT: Running analysis: MemorySSAAnalysis
> +; CHECK-O-NEXT: Running pass: GVNHoistPass on foo
> +; CHECK-O-NEXT: Running analysis: PostDominatorTreeAnalysis on foo
> +; CHECK-O-NEXT: Running analysis: MemoryDependenceAnalysis on foo
> +; CHECK-O-NEXT: Running analysis: PhiValuesAnalysis on foo
>  ; CHECK-O-NEXT: Running pass: SpeculativeExecutionPass
>  ; CHECK-O-NEXT: Running pass: JumpThreadingPass
>  ; CHECK-O-NEXT: Running analysis: LazyValueAnalysis
> @@ -169,23 +173,13 @@
>  ; CHECK-O-NEXT: Finished Loop pass manager run.
>  ; CHECK-Os-NEXT: Running pass: MergedLoadStoreMotionPass
>  ; CHECK-Os-NEXT: Running pass: GVN
> -; CHECK-Os-NEXT: Running analysis: MemoryDependenceAnalysis
> -; CHECK-Os-NEXT: Running analysis: PhiValuesAnalysis
>  ; CHECK-Oz-NEXT: Running pass: MergedLoadStoreMotionPass
>  ; CHECK-Oz-NEXT: Running pass: GVN
> -; CHECK-Oz-NEXT: Running analysis: MemoryDependenceAnalysis
> -; CHECK-Oz-NEXT: Running analysis: PhiValuesAnalysis
>  ; CHECK-O2-NEXT: Running pass: MergedLoadStoreMotionPass
>  ; CHECK-O2-NEXT: Running pass: GVN
> -; CHECK-O2-NEXT: Running analysis: MemoryDependenceAnalysis
> -; CHECK-O2-NEXT: Running analysis: PhiValuesAnalysis
>  ; CHECK-O3-NEXT: Running pass: MergedLoadStoreMotionPass
>  ; CHECK-O3-NEXT: Running pass: GVN
> -; CHECK-O3-NEXT: Running analysis: MemoryDependenceAnalysis
> -; CHECK-O3-NEXT: Running analysis: PhiValuesAnalysis
>  ; CHECK-O-NEXT: Running pass: MemCpyOptPass
> -; CHECK-O1-NEXT: Running analysis: MemoryDependenceAnalysis
> -; CHECK-O1-NEXT: Running analysis: PhiValuesAnalysis
>  ; CHECK-O-NEXT: Running pass: SCCPPass
>  ; CHECK-O-NEXT: Running pass: BDCEPass
>  ; CHECK-O-NEXT: Running analysis: DemandedBitsAnalysis
> @@ -201,7 +195,6 @@
>  ; CHECK-O-NEXT: Finished llvm::Function pass manager run.
>  ; CHECK-EP-SCALAR-LATE-NEXT: Running pass: NoOpFunctionPass
>  ; CHECK-O-NEXT: Running pass: ADCEPass
> -; CHECK-O-NEXT: Running analysis: PostDominatorTreeAnalysis
>  ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
>  ; CHECK-O-NEXT: Running pass: InstCombinePass
>  ; CHECK-EP-PEEPHOLE-NEXT: Running pass: NoOpFunctionPass
>
> Modified: llvm/trunk/test/Other/new-pm-thinlto-defaults.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/new-pm-thinlto-defaults.ll?rev=342387&r1=342386&r2=342387&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Other/new-pm-thinlto-defaults.ll (original)
> +++ llvm/trunk/test/Other/new-pm-thinlto-defaults.ll Mon Sep 17 05:24:55
> 2018
> @@ -106,6 +106,10 @@
>  ; CHECK-O-NEXT: Running pass: SROA
>  ; CHECK-O-NEXT: Running pass: EarlyCSEPass
>  ; CHECK-O-NEXT: Running analysis: MemorySSAAnalysis
> +; CHECK-O-NEXT: Running pass: GVNHoistPass on foo
> +; CHECK-O-NEXT: Running analysis: PostDominatorTreeAnalysis on foo
> +; CHECK-O-NEXT: Running analysis: MemoryDependenceAnalysis on foo
> +; CHECK-O-NEXT: Running analysis: PhiValuesAnalysis on foo
>  ; CHECK-O-NEXT: Running pass: SpeculativeExecutionPass
>  ; CHECK-O-NEXT: Running pass: JumpThreadingPass
>  ; CHECK-O-NEXT: Running analysis: LazyValueAnalysis
> @@ -151,23 +155,13 @@
>  ; CHECK-O-NEXT: Finished Loop pass manager run.
>  ; CHECK-Os-NEXT: Running pass: MergedLoadStoreMotionPass
>  ; CHECK-Os-NEXT: Running pass: GVN
> -; CHECK-Os-NEXT: Running analysis: MemoryDependenceAnalysis
> -; CHECK-Os-NEXT: Running analysis: PhiValuesAnalysis
>  ; CHECK-Oz-NEXT: Running pass: MergedLoadStoreMotionPass
>  ; CHECK-Oz-NEXT: Running pass: GVN
> -; CHECK-Oz-NEXT: Running analysis: MemoryDependenceAnalysis
> -; CHECK-Oz-NEXT: Running analysis: PhiValuesAnalysis
>  ; CHECK-O2-NEXT: Running pass: MergedLoadStoreMotionPass
>  ; CHECK-O2-NEXT: Running pass: GVN
> -; CHECK-O2-NEXT: Running analysis: MemoryDependenceAnalysis
> -; CHECK-O2-NEXT: Running analysis: PhiValuesAnalysis
>  ; CHECK-O3-NEXT: Running pass: MergedLoadStoreMotionPass
>  ; CHECK-O3-NEXT: Running pass: GVN
> -; CHECK-O3-NEXT: Running analysis: MemoryDependenceAnalysis
> -; CHECK-O3-NEXT: Running analysis: PhiValuesAnalysis
>  ; CHECK-O-NEXT: Running pass: MemCpyOptPass
> -; CHECK-O1-NEXT: Running analysis: MemoryDependenceAnalysis
> -; CHECK-O1-NEXT: Running analysis: PhiValuesAnalysis
>  ; CHECK-O-NEXT: Running pass: SCCPPass
>  ; CHECK-O-NEXT: Running pass: BDCEPass
>  ; CHECK-O-NEXT: Running analysis: DemandedBitsAnalysis
> @@ -181,7 +175,6 @@
>  ; CHECK-O-NEXT: Running pass: LCSSAPass
>  ; CHECK-O-NEXT: Finished llvm::Function pass manager run
>  ; CHECK-O-NEXT: Running pass: ADCEPass
> -; CHECK-O-NEXT: Running analysis: PostDominatorTreeAnalysis
>  ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
>  ; CHECK-O-NEXT: Running pass: InstCombinePass
>  ; CHECK-O-NEXT: Finished llvm::Function pass manager run.
>
> Modified: llvm/trunk/test/Other/opt-O2-pipeline.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/opt-O2-pipeline.ll?rev=342387&r1=342386&r2=342387&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Other/opt-O2-pipeline.ll (original)
> +++ llvm/trunk/test/Other/opt-O2-pipeline.ll Mon Sep 17 05:24:55 2018
> @@ -59,6 +59,12 @@
>  ; CHECK-NEXT:         Function Alias Analysis Results
>  ; CHECK-NEXT:         Memory SSA
>  ; CHECK-NEXT:         Early CSE w/ MemorySSA
> +; CHECK-NEXT:         Post-Dominator Tree Construction
> +; CHECK-NEXT:         Basic Alias Analysis (stateless AA impl)
> +; CHECK-NEXT:         Function Alias Analysis Results
> +; CHECK-NEXT:         Phi Values Analysis
> +; CHECK-NEXT:         Memory Dependence Analysis
> +; CHECK-NEXT:         Early GVN Hoisting of Expressions
>  ; CHECK-NEXT:         Speculatively execute instructions if target has
> divergent branches
>  ; CHECK-NEXT:         Basic Alias Analysis (stateless AA impl)
>  ; CHECK-NEXT:         Function Alias Analysis Results
>
> Modified: llvm/trunk/test/Other/opt-O3-pipeline.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/opt-O3-pipeline.ll?rev=342387&r1=342386&r2=342387&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Other/opt-O3-pipeline.ll (original)
> +++ llvm/trunk/test/Other/opt-O3-pipeline.ll Mon Sep 17 05:24:55 2018
> @@ -62,6 +62,12 @@
>  ; CHECK-NEXT:         Function Alias Analysis Results
>  ; CHECK-NEXT:         Memory SSA
>  ; CHECK-NEXT:         Early CSE w/ MemorySSA
> +; CHECK-NEXT:         Post-Dominator Tree Construction
> +; CHECK-NEXT:         Basic Alias Analysis (stateless AA impl)
> +; CHECK-NEXT:         Function Alias Analysis Results
> +; CHECK-NEXT:         Phi Values Analysis
> +; CHECK-NEXT:         Memory Dependence Analysis
> +; CHECK-NEXT:         Early GVN Hoisting of Expressions
>  ; CHECK-NEXT:         Speculatively execute instructions if target has
> divergent branches
>  ; CHECK-NEXT:         Basic Alias Analysis (stateless AA impl)
>  ; CHECK-NEXT:         Function Alias Analysis Results
>
> Modified: llvm/trunk/test/Other/opt-Os-pipeline.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/opt-Os-pipeline.ll?rev=342387&r1=342386&r2=342387&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Other/opt-Os-pipeline.ll (original)
> +++ llvm/trunk/test/Other/opt-Os-pipeline.ll Mon Sep 17 05:24:55 2018
> @@ -59,6 +59,12 @@
>  ; CHECK-NEXT:         Function Alias Analysis Results
>  ; CHECK-NEXT:         Memory SSA
>  ; CHECK-NEXT:         Early CSE w/ MemorySSA
> +; CHECK-NEXT:         Post-Dominator Tree Construction
> +; CHECK-NEXT:         Basic Alias Analysis (stateless AA impl)
> +; CHECK-NEXT:         Function Alias Analysis Results
> +; CHECK-NEXT:         Phi Values Analysis
> +; CHECK-NEXT:         Memory Dependence Analysis
> +; CHECK-NEXT:         Early GVN Hoisting of Expressions
>  ; CHECK-NEXT:         Speculatively execute instructions if target has
> divergent branches
>  ; CHECK-NEXT:         Basic Alias Analysis (stateless AA impl)
>  ; CHECK-NEXT:         Function Alias Analysis Results
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20181002/fbf6820e/attachment-0001.html>