[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

2021-11-15 Thread Arthur Eubanks via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG19867de9e793: [NewPM] Only invalidate modified 
functions analyses in CGSCC passes + turn on… (authored by aeubanks).

Changed prior to commit:
  https://reviews.llvm.org/D113304?vs=385642=387410#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113304/new/

https://reviews.llvm.org/D113304

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Analysis/CGSCCPassManager.cpp
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-eager-invalidate.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-pgo-preinline.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/FunctionAttrs/invalidate.ll
  llvm/test/Transforms/Inline/analysis-invalidation.ll
  llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll

Index: llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
===
--- llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
+++ llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
@@ -8,11 +8,11 @@
 ;
 ; CHECK: Running pass: InlinerPass on (test1_f, test1_g, test1_h)
 ; CHECK: Running analysis: DominatorTreeAnalysis on test1_f
-; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_f
 ; CHECK: Invalidating analysis: LoopAnalysis on test1_f
 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_f
 ; CHECK: Invalidating analysis: BlockFrequencyAnalysis on test1_f
+; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: LoopAnalysis on test1_g
 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_g
@@ -29,7 +29,6 @@
 ; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_h
 ; CHECK-NOT: Invalidating analysis:
 ; CHECK: Running pass: DominatorTreeVerifierPass on test1_f
-; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_f
 
 ; An external function used to control branches.
 declare i1 @flag()
Index: llvm/test/Transforms/Inline/analysis-invalidation.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/analysis-invalidation.ll
@@ -0,0 +1,17 @@
+; RUN: opt -passes=inline < %s -disable-output -debug-pass-manager 2>&1 | FileCheck %s
+
+; We shouldn't invalidate any function analyses on g since it's never modified.
+
+; CHECK-NOT: Invalidating{{.*}} on g
+; CHECK: Invalidating{{.*}} on f
+; CHECK-NOT: Invalidating{{.*}} on g
+
+define void @f() noinline {
+  call void @g()
+  ret void
+}
+
+define void @g() alwaysinline {
+  call void @f()
+  ret void
+}
Index: llvm/test/Transforms/FunctionAttrs/invalidate.ll
===
--- /dev/null
+++ llvm/test/Transforms/FunctionAttrs/invalidate.ll
@@ -0,0 +1,25 @@
+; RUN: opt -passes='function(require),cgscc(function-attrs)' -disable-output < %s -debug-pass-manager 2>&1 | FileCheck %s
+
+; CHECK: Running pass: PostOrderFunctionAttrsPass on (f)
+; CHECK: Invalidating analysis: NoOpFunctionAnalysis on f
+; CHECK-NOT: Invalidating analysis: NoOpFunctionAnalysis on h
+; CHECK: Invalidating analysis: NoOpFunctionAnalysis on g
+; CHECK-NOT: Invalidating analysis: NoOpFunctionAnalysis on h
+; CHECK: Running pass: PostOrderFunctionAttrsPass on (g)
+; CHECK: Running pass: PostOrderFunctionAttrsPass on (h)
+
+declare i32 @e(i32(i32)*)
+
+define i32 @f(i32 %a) {
+  ret i32 %a
+}
+
+define i32 @g(i32 %b) {
+  %c = call i32 @f(i32 %b)
+  ret i32 %c
+}
+
+define i32 @h(i32 %b) {
+  %c = call i32 @e(i32(i32)* @f)
+  ret i32 %c
+}
Index: llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
@@ -1,26 +1,26 @@
 ; Validate ThinLTO prelink pipeline when we have Sample PGO
 ;
-; RUN: opt -disable-verify -verify-cfg-preserved=0 -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=0 -eagerly-invalidate-analyses=0 -debug-pass-manager \
 ; RUN: -pgo-kind=pgo-sample-use-pipeline -profile-file='%S/Inputs/new-pm-thinlto-samplepgo-defaults.prof' \
 ; RUN: -passes='thinlto-pre-link' -S %s 2>&1 \
 ; RUN:

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

2021-11-09 Thread Nikita Popov via Phabricator via cfe-commits 
nikic accepted this revision.
nikic added a comment.

LGTM as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113304/new/

https://reviews.llvm.org/D113304

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

2021-11-09 Thread Alina Sbirlea via Phabricator via cfe-commits 
asbirlea accepted this revision.
asbirlea added a comment.
This revision is now accepted and ready to land.

LGTM, but please wait to see if @nikic has additional comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113304/new/

https://reviews.llvm.org/D113304

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

2021-11-08 Thread Arthur Eubanks via Phabricator via cfe-commits 
aeubanks added inline comments.



Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:1017
+// invalidate analyses for all functions in this SCC later.
+FAM.invalidate(F, PreservedAnalyses::none());
   }

mtrofin wrote:
> Should we do this if !Changed? Actually, if this function did not change 
> (`Changed` is per cgscc)
we already bail out at line 966 if this function hasn't been changed


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113304/new/

https://reviews.llvm.org/D113304

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

2021-11-08 Thread Mircea Trofin via Phabricator via cfe-commits 
mtrofin added inline comments.



Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:1017
+// invalidate analyses for all functions in this SCC later.
+FAM.invalidate(F, PreservedAnalyses::none());
   }

Should we do this if !Changed? Actually, if this function did not change 
(`Changed` is per cgscc)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113304/new/

https://reviews.llvm.org/D113304

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

2021-11-08 Thread Arthur Eubanks via Phabricator via cfe-commits 
aeubanks updated this revision to Diff 385642.
aeubanks added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113304/new/

https://reviews.llvm.org/D113304

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Analysis/CGSCCPassManager.cpp
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-eager-invalidate.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-pgo-preinline.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/FunctionAttrs/invalidate.ll
  llvm/test/Transforms/Inline/analysis-invalidation.ll
  llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll

Index: llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
===
--- llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
+++ llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
@@ -8,11 +8,11 @@
 ;
 ; CHECK: Running pass: InlinerPass on (test1_f, test1_g, test1_h)
 ; CHECK: Running analysis: DominatorTreeAnalysis on test1_f
-; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_f
 ; CHECK: Invalidating analysis: LoopAnalysis on test1_f
 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_f
 ; CHECK: Invalidating analysis: BlockFrequencyAnalysis on test1_f
+; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: LoopAnalysis on test1_g
 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_g
@@ -29,7 +29,6 @@
 ; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_h
 ; CHECK-NOT: Invalidating analysis:
 ; CHECK: Running pass: DominatorTreeVerifierPass on test1_f
-; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_f
 
 ; An external function used to control branches.
 declare i1 @flag()
Index: llvm/test/Transforms/Inline/analysis-invalidation.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/analysis-invalidation.ll
@@ -0,0 +1,17 @@
+; RUN: opt -passes=inline < %s -disable-output -debug-pass-manager 2>&1 | FileCheck %s
+
+; We shouldn't invalidate any function analyses on g since it's never modified.
+
+; CHECK-NOT: Invalidating{{.*}} on g
+; CHECK: Invalidating{{.*}} on f
+; CHECK-NOT: Invalidating{{.*}} on g
+
+define void @f() noinline {
+  call void @g()
+  ret void
+}
+
+define void @g() alwaysinline {
+  call void @f()
+  ret void
+}
Index: llvm/test/Transforms/FunctionAttrs/invalidate.ll
===
--- /dev/null
+++ llvm/test/Transforms/FunctionAttrs/invalidate.ll
@@ -0,0 +1,25 @@
+; RUN: opt -passes='function(require),cgscc(function-attrs)' -disable-output < %s -debug-pass-manager 2>&1 | FileCheck %s
+
+; CHECK: Running pass: PostOrderFunctionAttrsPass on (f)
+; CHECK: Invalidating analysis: NoOpFunctionAnalysis on f
+; CHECK-NOT: Invalidating analysis: NoOpFunctionAnalysis on h
+; CHECK: Invalidating analysis: NoOpFunctionAnalysis on g
+; CHECK-NOT: Invalidating analysis: NoOpFunctionAnalysis on h
+; CHECK: Running pass: PostOrderFunctionAttrsPass on (g)
+; CHECK: Running pass: PostOrderFunctionAttrsPass on (h)
+
+declare i32 @e(i32(i32)*)
+
+define i32 @f(i32 %a) {
+  ret i32 %a
+}
+
+define i32 @g(i32 %b) {
+  %c = call i32 @f(i32 %b)
+  ret i32 %c
+}
+
+define i32 @h(i32 %b) {
+  %c = call i32 @e(i32(i32)* @f)
+  ret i32 %c
+}
Index: llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
@@ -1,26 +1,26 @@
 ; Validate ThinLTO prelink pipeline when we have Sample PGO
 ;
-; RUN: opt -disable-verify -verify-cfg-preserved=0 -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=0 -eagerly-invalidate-analyses=0 -debug-pass-manager \
 ; RUN: -pgo-kind=pgo-sample-use-pipeline -profile-file='%S/Inputs/new-pm-thinlto-samplepgo-defaults.prof' \
 ; RUN: -passes='thinlto-pre-link' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-O1
-; RUN: opt -disable-verify -verify-cfg-preserved=0 -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=0 -eagerly-invalidate-analyses=0 -debug-pass-manager \
 ; RUN:

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

2021-11-08 Thread Alina Sbirlea via Phabricator via cfe-commits 
asbirlea added a comment.

Thank you for the additional clarifications in the comments and descriptions!

The invalidations are essentially postponed until after the function 
simplification pipeline with this patch.
Before, everything was invalidated in the SCC pass even if only one function 
was touched, and nothing was invalidated after the function passes. With this 
patch, only what is touched is invalidated in the SCC pass, then function 
passes that follow find analyses available (where functions were unmodified), 
and only afterwards all function analyses are invalidated in the 
ModuleToFunction adaptor. The balance is from turning on both D113196 
 and D100917 
.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113304/new/

https://reviews.llvm.org/D113304

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

2021-11-08 Thread Arthur Eubanks via Phabricator via cfe-commits 
aeubanks added a comment.

In D113304#3112937 , @nikic wrote:

> I don't think I fully understand the interaction this has with eager 
> invalidation. I would have expected that if we eagerly invalidate, then this 
> fine-grained invalidation wouldn't make much of a difference, because we 
> invalidate anyway after processing a function.

if we've already calculated e.g. DT for all functions in an SCC prior to 
visiting the SCC, currently if an SCC pass changes any of the functions, we'd 
invalidate analyses for all the functions in the SCC, meaning function passes 
in the SCC adaptor would have to recalculate. but if we do this fine-grained 
invalidation we won't have to recalculate DT in the function passes for 
functions that weren't modified in the SCC pass

I'll add this to the description


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113304/new/

https://reviews.llvm.org/D113304

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

2021-11-08 Thread Arthur Eubanks via Phabricator via cfe-commits 
aeubanks updated this revision to Diff 385539.
aeubanks added a comment.

address comments
[argpromo] only invalidate direct callers that call the function, not if the 
function is a parameter


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113304/new/

https://reviews.llvm.org/D113304

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Analysis/CGSCCPassManager.cpp
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-eager-invalidate.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-pgo-preinline.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/FunctionAttrs/invalidate.ll
  llvm/test/Transforms/Inline/analysis-invalidation.ll
  llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll

Index: llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
===
--- llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
+++ llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
@@ -8,11 +8,11 @@
 ;
 ; CHECK: Running pass: InlinerPass on (test1_f, test1_g, test1_h)
 ; CHECK: Running analysis: DominatorTreeAnalysis on test1_f
-; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_f
 ; CHECK: Invalidating analysis: LoopAnalysis on test1_f
 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_f
 ; CHECK: Invalidating analysis: BlockFrequencyAnalysis on test1_f
+; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: LoopAnalysis on test1_g
 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_g
@@ -29,7 +29,6 @@
 ; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_h
 ; CHECK-NOT: Invalidating analysis:
 ; CHECK: Running pass: DominatorTreeVerifierPass on test1_f
-; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_f
 
 ; An external function used to control branches.
 declare i1 @flag()
Index: llvm/test/Transforms/Inline/analysis-invalidation.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/analysis-invalidation.ll
@@ -0,0 +1,17 @@
+; RUN: opt -passes=inline < %s -disable-output -debug-pass-manager 2>&1 | FileCheck %s
+
+; We shouldn't invalidate any function analyses on g since it's never modified.
+
+; CHECK-NOT: Invalidating{{.*}} on g
+; CHECK: Invalidating{{.*}} on f
+; CHECK-NOT: Invalidating{{.*}} on g
+
+define void @f() noinline {
+  call void @g()
+  ret void
+}
+
+define void @g() alwaysinline {
+  call void @f()
+  ret void
+}
Index: llvm/test/Transforms/FunctionAttrs/invalidate.ll
===
--- /dev/null
+++ llvm/test/Transforms/FunctionAttrs/invalidate.ll
@@ -0,0 +1,25 @@
+; RUN: opt -passes='function(require),cgscc(function-attrs)' -disable-output < %s -debug-pass-manager 2>&1 | FileCheck %s
+
+; CHECK: Running pass: PostOrderFunctionAttrsPass on (f)
+; CHECK: Invalidating analysis: NoOpFunctionAnalysis on f
+; CHECK-NOT: Invalidating analysis: NoOpFunctionAnalysis on h
+; CHECK: Invalidating analysis: NoOpFunctionAnalysis on g
+; CHECK-NOT: Invalidating analysis: NoOpFunctionAnalysis on h
+; CHECK: Running pass: PostOrderFunctionAttrsPass on (g)
+; CHECK: Running pass: PostOrderFunctionAttrsPass on (h)
+
+declare i32 @e(i32(i32)*)
+
+define i32 @f(i32 %a) {
+  ret i32 %a
+}
+
+define i32 @g(i32 %b) {
+  %c = call i32 @f(i32 %b)
+  ret i32 %c
+}
+
+define i32 @h(i32 %b) {
+  %c = call i32 @e(i32(i32)* @f)
+  ret i32 %c
+}
Index: llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
@@ -1,26 +1,26 @@
 ; Validate ThinLTO prelink pipeline when we have Sample PGO
 ;
-; RUN: opt -disable-verify -verify-cfg-preserved=0 -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=0 -eagerly-invalidate-analyses=0 -debug-pass-manager \
 ; RUN: -pgo-kind=pgo-sample-use-pipeline -profile-file='%S/Inputs/new-pm-thinlto-samplepgo-defaults.prof' \
 ; RUN: -passes='thinlto-pre-link' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-O1
-; RUN: opt -disable-verify -verify-cfg-preserved=0 -debug-pass-manager \
+; RUN: opt -disable-verify

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

2021-11-08 Thread Arthur Eubanks via Phabricator via cfe-commits 
aeubanks added inline comments.



Comment at: llvm/lib/Transforms/IPO/FunctionAttrs.cpp:1845
+  if (auto *Call = dyn_cast(U))
+FAM.invalidate(*Call->getParent()->getParent(), FuncPA);
+}

nikic wrote:
> Do we need to worry about indirect references here, like a call through a 
> bitcast? For the ArgPromotion case that's not possible, but I'm not so sure 
> here.
I suppose it's possible for analyses to look at all references to other 
functions and look at attributes of those functions, but I can't really imagine 
that happening in practice. AFAICT everything that looks at function attributes 
only does so for direct calls.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113304/new/

https://reviews.llvm.org/D113304

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

2021-11-05 Thread Nikita Popov via Phabricator via cfe-commits 
nikic added a comment.

I don't think I fully understand the interaction this has with eager 
invalidation. I would have expected that if we eagerly invalidate, then this 
fine-grained invalidation wouldn't make much of a difference, because we 
invalidate anyway after processing a function.




Comment at: llvm/lib/Transforms/IPO/ArgumentPromotion.cpp:1051
+  for (auto *U : NewF->users()) {
+auto *UserF = cast(U)->getParent()->getParent();
+FAM.invalidate(*UserF, FuncPA);





Comment at: llvm/lib/Transforms/IPO/FunctionAttrs.cpp:1845
+  if (auto *Call = dyn_cast(U))
+FAM.invalidate(*Call->getParent()->getParent(), FuncPA);
+}

Do we need to worry about indirect references here, like a call through a 
bitcast? For the ArgPromotion case that's not possible, but I'm not so sure 
here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113304/new/

https://reviews.llvm.org/D113304

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

2021-11-05 Thread Arthur Eubanks via Phabricator via cfe-commits 
aeubanks created this revision.
Herald added subscribers: ormris, wenlei, steven_wu, hiraditya, eraman.
Herald added a reviewer: ctetreau.
aeubanks requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Previously, any change in any function in an SCC would cause all
analyses for all functions in the SCC to be invalidated. With this
change, we now manually invalidate analyses for functions we modify,
then let the pass manager know that all function analyses should be
preserved since we've already handled function analysis invalidation.

So far this only touches the inliner, argpromotion, function-attrs, and
updateCGAndAnalysisManager(), since they are the most used.

This is part of an effort to investigate running the function
simplification pipeline less on functions we visit multiple times in the
inliner pipeline.

However, this causes major memory regressions especially on larger IR.
To counteract this, turn on the option to eagerly invalidate function
analyses. This invalidates analyses on functions immediately after
they're processed in a module or scc to function adaptor for specific
parts of the pipeline.

Overall this has mostly positive effects on compile time and positive effects 
on memory usage.
https://llvm-compile-time-tracker.com/compare.php?from=7f627596977624730f9298a1b69883af1555765e=39e824e0d3ca8a517502f13032dfa67304841c90=instructions
https://llvm-compile-time-tracker.com/compare.php?from=7f627596977624730f9298a1b69883af1555765e=39e824e0d3ca8a517502f13032dfa67304841c90=max-rss


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113304

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Analysis/CGSCCPassManager.cpp
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-eager-invalidate.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-pgo-preinline.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/FunctionAttrs/invalidate.ll
  llvm/test/Transforms/Inline/analysis-invalidation.ll
  llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll

Index: llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
===
--- llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
+++ llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
@@ -8,11 +8,11 @@
 ;
 ; CHECK: Running pass: InlinerPass on (test1_f, test1_g, test1_h)
 ; CHECK: Running analysis: DominatorTreeAnalysis on test1_f
-; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_f
 ; CHECK: Invalidating analysis: LoopAnalysis on test1_f
 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_f
 ; CHECK: Invalidating analysis: BlockFrequencyAnalysis on test1_f
+; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: LoopAnalysis on test1_g
 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_g
@@ -29,7 +29,6 @@
 ; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_h
 ; CHECK-NOT: Invalidating analysis:
 ; CHECK: Running pass: DominatorTreeVerifierPass on test1_f
-; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_f
 
 ; An external function used to control branches.
 declare i1 @flag()
Index: llvm/test/Transforms/Inline/analysis-invalidation.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/analysis-invalidation.ll
@@ -0,0 +1,17 @@
+; RUN: opt -passes=inline < %s -disable-output -debug-pass-manager 2>&1 | FileCheck %s
+
+; We shouldn't invalidate any function analyses on g since it's never modified.
+
+; CHECK-NOT: Invalidating{{.*}} on g
+; CHECK: Invalidating{{.*}} on f
+; CHECK-NOT: Invalidating{{.*}} on g
+
+define void @f() noinline {
+  call void @g()
+  ret void
+}
+
+define void @g() alwaysinline {
+  call void @f()
+  ret void
+}
Index: llvm/test/Transforms/FunctionAttrs/invalidate.ll
===
--- /dev/null
+++ llvm/test/Transforms/FunctionAttrs/invalidate.ll
@@ -0,0 +1,15 @@
+; RUN: opt -passes='function(require),cgscc(function-attrs)' -disable-output < %s -debug-pass-manager 2>&1 | FileCheck %s
+
+; CHECK: Running pass: PostOrderFunctionAttrsPass on (f)
+; CHECK: Invalidating analysis: NoOpFunctionAnalysis on f
+; CHECK: Invalidating analysis: