[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

tejohnson added a comment. In D155997#4570562 , @aeubanks wrote: > can we try not gating this on PGO as suggested? minimizing differences > between pipelines is nice, and as mentioned it'll help with other cases Sorry for the delay, was OOO for a bit

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

aeubanks added a comment. can we try not gating this on PGO as suggested? minimizing differences between pipelines is nice, and as mentioned it'll help with other cases Comment at: llvm/test/Transforms/PhaseOrdering/simplifycfg-speculate-blocks.ll:1 +;; Check that SimplifyCFG

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

tejohnson added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155997/new/ https://reviews.llvm.org/D155997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

tejohnson updated this revision to Diff 545318. tejohnson marked an inline comment as done. tejohnson added a comment. Address test comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155997/new/ https://reviews.llvm.org/D155997 Files:

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

tejohnson marked 2 inline comments as done. tejohnson added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/simplifycfg-speculate-blocks.ll:83 +!4 = !{i32 1, !"wchar_size", i32 4} +!5 = !{i32 8, !"PIC Level", i32 2} +!6 = !{i32 7, !"PIE Level", i32 2}

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

tejohnson added a comment. In D155997#4543156 , @nikic wrote: > I'm not a fan of the PGO conditional behavior here. I'd prefer to disable > speculation unconditionally if that is feasible. This is basically what > D153392

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

nikic added a comment. I'm not a fan of the PGO conditional behavior here. I'd prefer to disable speculation unconditionally if that is feasible. This is basically what D153392 did. While the specific test case there was addressed in a different way, if we

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

aeubanks added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/simplifycfg-speculate-blocks.ll:1 +;; Check that SimplifyCFG does not attempt speculation until after PGO is +;; annotated in the IR, and then does not perform it when unprofitable.

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

tejohnson updated this revision to Diff 544109. tejohnson added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155997/new/ https://reviews.llvm.org/D155997 Files: clang/test/CodeGen/pgo-sample-preparation.c

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

tejohnson added a comment. In D155997#4529861 , @aeubanks wrote: > the pipeline change and simplifycfg change should be split into two changes SimplifyCFG change split into D156194 . Repository: rG LLVM Github

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

aeubanks added inline comments. Comment at: llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll:3 ; RUN: opt < %s -S -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -bonus-inst-threshold=10 | FileCheck %s +; RUN: opt < %s -S -passes='simplifycfg'

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

tejohnson added inline comments. Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:277 +static cl::opt AlwaysAllowSimplifyCFGSpeculation( +"always-allow-simplify-cfg-speculation", cl::init(false), cl::Hidden, aeubanks wrote: > can we just drop the flag

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

aeubanks added a comment. the pipeline change and simplifycfg change should be split into two changes Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:277 +static cl::opt AlwaysAllowSimplifyCFGSpeculation( +"always-allow-simplify-cfg-speculation", cl::init(false),

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

tejohnson created this revision. tejohnson added reviewers: aeubanks, nikic. Herald added subscribers: khei4, wlei, StephenFan, wenlei, hiraditya. Herald added a project: All. tejohnson requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-commits.