[llvm] r296759 - Re-apply "[GVNHoist] Move GVNHoist to function simplification part of pipeline."

[David Green via llvm-commits]

Hello,

I see that this was reverted, but felt I should still mention that we are unfortunately seeing some pretty significant performance regressions from this change. Some as high as 50%. There must be some micro-architectural funny business going on in those cases to make the change so large, but it looks like their were previously loads/stores that are no longer pulled out of a hot loop.

This all comes from propriety benchmarks, so when I get the chance, I'll attempt to make you something that reproduces the problems.

Cheers
Dave




From: llvm-commits <llvm-commits-bounces at lists.llvm.org> on behalf of Geoff Berry via llvm-commits <llvm-commits at lists.llvm.org>
Sent: 02 March 2017 16:16
To: llvm-commits at lists.llvm.org
Subject: [llvm] r296759 - Re-apply "[GVNHoist] Move GVNHoist to function simplification part of pipeline."
    
Author: gberry
Date: Thu Mar  2 10:16:47 2017
New Revision: 296759

URL: http://llvm.org/viewvc/llvm-project?rev=296759&view=rev
Log:
Re-apply "[GVNHoist] Move GVNHoist to function simplification part of pipeline."

This re-applies r289696, which caused TSan perf regression, which has
since been addressed in separate changes (see PR for details).

See PR31382.

Added:
    llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll
Modified:
    llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp

Modified: llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp
URL:  http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp?rev=296759&r1=296758&r2=296759&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp Thu Mar  2 10:16:47 2017
@@ -244,8 +244,6 @@ void PassManagerBuilder::populateFunctio
   FPM.add(createCFGSimplificationPass());
   FPM.add(createSROAPass());
   FPM.add(createEarlyCSEPass());
-  if(EnableGVNHoist)
-    FPM.add(createGVNHoistPass());
   FPM.add(createLowerExpectIntrinsicPass());
 }
 
@@ -290,6 +288,8 @@ void PassManagerBuilder::addFunctionSimp
   // Break up aggregate allocas, using SSAUpdater.
   MPM.add(createSROAPass());
   MPM.add(createEarlyCSEPass());              // Catch trivial redundancies
+  if(EnableGVNHoist)
+    MPM.add(createGVNHoistPass());
   // Speculative execution if the target has divergent branches; otherwise nop.
   MPM.add(createSpeculativeExecutionIfHasBranchDivergencePass());
   MPM.add(createJumpThreadingPass());         // Thread jumps.

Added: llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll
URL:  http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll?rev=296759&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll (added)
+++ llvm/trunk/test/Transforms/GVNHoist/hoist-inline.ll Thu Mar  2 10:16:47 2017
@@ -0,0 +1,38 @@
+; RUN: opt -S -O2 < %s | FileCheck %s
+
+; Check that the inlined loads are hoisted.
+; CHECK-LABEL: define i32 @fun(
+; CHECK-LABEL: entry:
+; CHECK: load i32, i32* @A
+; CHECK: if.then:
+
+ at A = external global i32
+ at B = external global i32
+ at C = external global i32
+
+define i32 @loadA() {
+   %a = load i32, i32* @A
+   ret i32 %a
+}
+
+define i32 @fun(i1 %c) {
+entry:
+  br i1 %c, label %if.then, label %if.else
+
+if.then:
+  store i32 1, i32* @B
+  %call1 = call i32 @loadA()
+  store i32 2, i32* @C
+  br label %if.endif
+
+if.else:
+  store i32 2, i32* @C
+  %call2 = call i32 @loadA()
+  store i32 1, i32* @B
+  br label %if.endif
+
+if.endif:
+  %ret = phi i32 [ %call1, %if.then ], [ %call2, %if.else ]
+  ret i32 %ret
+}
+


_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits