This is an archive of the discontinued LLVM Phabricator instance.

[FuncSpec] Replace LoopInfo with BlockFrequencyInfo.
ClosedPublic

Authored by labrinea on May 11 2023, 8:32 AM.

Details

Summary

Using AvgLoopIters on any loop is too imprecise making the cost model favor users inside loop nests regardless of the actual tripcount.

Compile times -O3

benchmarknspecs beforenspecs afterinstrCnt delta %
ClamAV55+0.006
7zip-0.031
tramp3d-v4-0.043
kimwitu++-0.156
sqlite32-0.571
mafft-0.029
lencod+0.029
SPASS22-0.038
consumer-typeset-0.045
Bullet1+0.055
geomean-0.083

Compile times LTO

benchmarknspecs beforenspecs afterinstrCnt delta %
ClamAV1-0.159
7zip-0.023
tramp3d-v4-0.018
kimwitu+++0.016
sqlite321+0.357
mafft+0.029
lencod0
SPASS1-0.283
consumer-typeset1+0.539
Bullet+0.013
geomean+0.047

Diff Detail

Event Timeline

labrinea created this revision.May 11 2023, 8:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 11 2023, 8:32 AM
labrinea requested review of this revision.May 11 2023, 8:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 11 2023, 8:32 AM
mingmingl added inline comments.May 11 2023, 9:57 AM
llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
599–606

(Hi! I notice this and was trying to get some generated function specialized so would like to share some thoughts :) )

Function spec cost is calculated as Metrics.NumInsts * InlineConstants::getInstrCost() (i.e., without TTI per instruction cost).
I wonder if it would make more sense to make per-instruction cost calculation consistent.

Also it seems Weight could give a big bonus for functions in a hot inner-loop with PGO, wonder how it affects code size in PGO case.

labrinea added inline comments.May 11 2023, 11:13 AM
llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
599–606

The Specialization cost estimation is wrong and I am planning to fix that in later patches. Metrics.NumInsts already contains TargetTransformInfo::TCK_CodeSize. Regarding the weight, right now it only favors basic blocks with higher frequency than the entry (in other words, loops). In future patches I want to tweak the weight calculation as <SomeConstant> x <BlockFreq> / <EntrFreq> in order to allow conditional code to be accounted for the Bonus calculation. I haven't measured PGO configurations thoroughly, but I am planning radical changes to the cost model in the near future as enabling specializations of literal constants is becoming necessity and right now is too heavy weight.

Interesting. Have you measured with spec2017?

Interesting. Have you measured with spec2017?

Hi. I measured (without PGO) but didn't see any changes. The main drive for this change is to enable specialization of literal constants (D150649), which would otherwse trigger too often with LoopInfo.

Interesting. Have you measured with spec2017?

Hi. I measured (without PGO) but didn't see any changes. The main drive for this change is to enable specialization of literal constants (D150649), which would otherwse trigger too often with LoopInfo.

Do you measured it with fulllto? If yes, I feel the change looks good.

Interesting. Have you measured with spec2017?

Hi. I measured (without PGO) but didn't see any changes. The main drive for this change is to enable specialization of literal constants (D150649), which would otherwse trigger too often with LoopInfo.

Do you measured it with fulllto? If yes, I feel the change looks good.

Yes, both default -O3 and full LTO pipelines as shown on the description.

ChuanqiXu accepted this revision.May 16 2023, 3:17 AM

LGTM then. Thanks! Let's give some time for other reviewers.

This revision is now accepted and ready to land.May 16 2023, 3:17 AM
This revision was landed with ongoing or failed builds.May 22 2023, 9:50 AM
This revision was automatically updated to reflect the committed changes.

Hello,

The following starts crashing with this patch:

opt -passes="ipsccp,function(adce)" -mtriple=hexagon bbi-82827.ll -o /dev/null

Result:

PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: ../../main-github/llvm/build-all/bin/opt -passes=ipsccp,function(adce) -mtriple=hexagon bbi-82827.ll -o /dev/null
 #0 0x00005590a334c8b7 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (../../main-github/llvm/build-all/bin/opt+0x2d978b7)
 #1 0x00005590a334a5de llvm::sys::RunSignalHandlers() (../../main-github/llvm/build-all/bin/opt+0x2d955de)
 #2 0x00005590a334cf4f SignalHandler(int) Signals.cpp:0:0
 #3 0x00007f396568d630 __restore_rt sigaction.c:0:0
 #4 0x00005590a2c565a1 llvm::DebugLoc::get() const (../../main-github/llvm/build-all/bin/opt+0x26a15a1)
 #5 0x00005590a3db2f31 (anonymous namespace)::AggressiveDeadCodeElimination::markLive(llvm::Instruction*) ADCE.cpp:0:0
 #6 0x00005590a3dafca1 (anonymous namespace)::AggressiveDeadCodeElimination::performDeadCodeElimination() ADCE.cpp:0:0
 #7 0x00005590a3dae4a9 llvm::ADCEPass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0x37f94a9)
 #8 0x00005590a35590dd llvm::detail::PassModel<llvm::Function, llvm::ADCEPass, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0x2fa40dd)
 #9 0x00005590a2d2c314 llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0x2777314)
#10 0x00005590a11e95ed llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0xc345ed)
#11 0x00005590a2d306fe llvm::ModuleToFunctionPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (../../main-github/llvm/build-all/bin/opt+0x277b6fe)
#12 0x00005590a11e938d llvm::detail::PassModel<llvm::Module, llvm::ModuleToFunctionPassAdaptor, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (../../main-github/llvm/build-all/bin/opt+0xc3438d)
#13 0x00005590a2d2b4a4 llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (../../main-github/llvm/build-all/bin/opt+0x27764a4)
#14 0x00005590a0e2314e llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::PassPlugin>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool) (../../main-github/llvm/build-all/bin/opt+0x86e14e)
#15 0x00005590a0e3181e main (../../main-github/llvm/build-all/bin/opt+0x87c81e)
#16 0x00007f3962dc0555 __libc_start_main (/lib64/libc.so.6+0x22555)
#17 0x00005590a0e1d8a0 _start (../../main-github/llvm/build-all/bin/opt+0x8688a0)
Segmentation fault (core dumped)

bjope added a subscriber: bjope.May 24 2023, 7:39 AM

Hello,

The following starts crashing with this patch:

opt -passes="ipsccp,function(adce)" -mtriple=hexagon bbi-82827.ll -o /dev/null

Result:

PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: ../../main-github/llvm/build-all/bin/opt -passes=ipsccp,function(adce) -mtriple=hexagon bbi-82827.ll -o /dev/null
 #0 0x00005590a334c8b7 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (../../main-github/llvm/build-all/bin/opt+0x2d978b7)
 #1 0x00005590a334a5de llvm::sys::RunSignalHandlers() (../../main-github/llvm/build-all/bin/opt+0x2d955de)
 #2 0x00005590a334cf4f SignalHandler(int) Signals.cpp:0:0
 #3 0x00007f396568d630 __restore_rt sigaction.c:0:0
 #4 0x00005590a2c565a1 llvm::DebugLoc::get() const (../../main-github/llvm/build-all/bin/opt+0x26a15a1)
 #5 0x00005590a3db2f31 (anonymous namespace)::AggressiveDeadCodeElimination::markLive(llvm::Instruction*) ADCE.cpp:0:0
 #6 0x00005590a3dafca1 (anonymous namespace)::AggressiveDeadCodeElimination::performDeadCodeElimination() ADCE.cpp:0:0
 #7 0x00005590a3dae4a9 llvm::ADCEPass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0x37f94a9)
 #8 0x00005590a35590dd llvm::detail::PassModel<llvm::Function, llvm::ADCEPass, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0x2fa40dd)
 #9 0x00005590a2d2c314 llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0x2777314)
#10 0x00005590a11e95ed llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0xc345ed)
#11 0x00005590a2d306fe llvm::ModuleToFunctionPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (../../main-github/llvm/build-all/bin/opt+0x277b6fe)
#12 0x00005590a11e938d llvm::detail::PassModel<llvm::Module, llvm::ModuleToFunctionPassAdaptor, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (../../main-github/llvm/build-all/bin/opt+0xc3438d)
#13 0x00005590a2d2b4a4 llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (../../main-github/llvm/build-all/bin/opt+0x27764a4)
#14 0x00005590a0e2314e llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::PassPlugin>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool) (../../main-github/llvm/build-all/bin/opt+0x86e14e)
#15 0x00005590a0e3181e main (../../main-github/llvm/build-all/bin/opt+0x87c81e)
#16 0x00007f3962dc0555 __libc_start_main (/lib64/libc.so.6+0x22555)
#17 0x00005590a0e1d8a0 _start (../../main-github/llvm/build-all/bin/opt+0x8688a0)
Segmentation fault (core dumped)

Ping @labrinea

Can you take a look at this regression?

Hello,

The following starts crashing with this patch:

opt -passes="ipsccp,function(adce)" -mtriple=hexagon bbi-82827.ll -o /dev/null

Result:

PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: ../../main-github/llvm/build-all/bin/opt -passes=ipsccp,function(adce) -mtriple=hexagon bbi-82827.ll -o /dev/null
 #0 0x00005590a334c8b7 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (../../main-github/llvm/build-all/bin/opt+0x2d978b7)
 #1 0x00005590a334a5de llvm::sys::RunSignalHandlers() (../../main-github/llvm/build-all/bin/opt+0x2d955de)
 #2 0x00005590a334cf4f SignalHandler(int) Signals.cpp:0:0
 #3 0x00007f396568d630 __restore_rt sigaction.c:0:0
 #4 0x00005590a2c565a1 llvm::DebugLoc::get() const (../../main-github/llvm/build-all/bin/opt+0x26a15a1)
 #5 0x00005590a3db2f31 (anonymous namespace)::AggressiveDeadCodeElimination::markLive(llvm::Instruction*) ADCE.cpp:0:0
 #6 0x00005590a3dafca1 (anonymous namespace)::AggressiveDeadCodeElimination::performDeadCodeElimination() ADCE.cpp:0:0
 #7 0x00005590a3dae4a9 llvm::ADCEPass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0x37f94a9)
 #8 0x00005590a35590dd llvm::detail::PassModel<llvm::Function, llvm::ADCEPass, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0x2fa40dd)
 #9 0x00005590a2d2c314 llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0x2777314)
#10 0x00005590a11e95ed llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0xc345ed)
#11 0x00005590a2d306fe llvm::ModuleToFunctionPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (../../main-github/llvm/build-all/bin/opt+0x277b6fe)
#12 0x00005590a11e938d llvm::detail::PassModel<llvm::Module, llvm::ModuleToFunctionPassAdaptor, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (../../main-github/llvm/build-all/bin/opt+0xc3438d)
#13 0x00005590a2d2b4a4 llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (../../main-github/llvm/build-all/bin/opt+0x27764a4)
#14 0x00005590a0e2314e llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::PassPlugin>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool) (../../main-github/llvm/build-all/bin/opt+0x86e14e)
#15 0x00005590a0e3181e main (../../main-github/llvm/build-all/bin/opt+0x87c81e)
#16 0x00007f3962dc0555 __libc_start_main (/lib64/libc.so.6+0x22555)
#17 0x00005590a0e1d8a0 _start (../../main-github/llvm/build-all/bin/opt+0x8688a0)
Segmentation fault (core dumped)

Ping @labrinea

Can you take a look at this regression?

It also crashes when using opt -passes="ipsccp,print<postdomtree>" -mtriple=hexagon bbi-82827.ll -o /dev/null.
So it is something with the PostDominatorTree analysis getting messed up?

Both valgrind and ASan complains about memory problems. ASan says there is heap-use-after-free for stuff freed here

#1 0x47b6bd9 in llvm::DomTreeUpdater::forceFlushDeletedBB()
#2 0x47b649c in tryFlushDeletedBB
#3 0x47b649c in llvm::DomTreeUpdater::dropOutOfDateUpdates()
#4 0x174687e in llvm::DomTreeUpdater::~DomTreeUpdater()
#5 0x8111719 in runIPSCCP(llvm::Module&, llvm::DataLayout const&, llvm::AnalysisManager<llvm::Function>*, std::__1::function<llvm::BlockFrequencyInfo& (llvm::Function&)>, std::__1::function<llvm::TargetLibraryInfo const& (llvm::Function&)>, std::__1::function<llvm::TargetTransformInfo& (llvm::Function&)>, std::__1::function<llvm::AssumptionCache& (llvm::Function&)>, llvm::function_ref<llvm::AnalysisResultsForFn (llvm::Function&)>, bool)

I've also seen crashes for other passes than adce, e.g. gvnhoist.

bjope added subscribers: aeubanks, asbirlea.EditedMay 26 2023, 1:20 AM

Hello,

The following starts crashing with this patch:

opt -passes="ipsccp,function(adce)" -mtriple=hexagon bbi-82827.ll -o /dev/null

Result:

PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: ../../main-github/llvm/build-all/bin/opt -passes=ipsccp,function(adce) -mtriple=hexagon bbi-82827.ll -o /dev/null
 #0 0x00005590a334c8b7 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (../../main-github/llvm/build-all/bin/opt+0x2d978b7)
 #1 0x00005590a334a5de llvm::sys::RunSignalHandlers() (../../main-github/llvm/build-all/bin/opt+0x2d955de)
 #2 0x00005590a334cf4f SignalHandler(int) Signals.cpp:0:0
 #3 0x00007f396568d630 __restore_rt sigaction.c:0:0
 #4 0x00005590a2c565a1 llvm::DebugLoc::get() const (../../main-github/llvm/build-all/bin/opt+0x26a15a1)
 #5 0x00005590a3db2f31 (anonymous namespace)::AggressiveDeadCodeElimination::markLive(llvm::Instruction*) ADCE.cpp:0:0
 #6 0x00005590a3dafca1 (anonymous namespace)::AggressiveDeadCodeElimination::performDeadCodeElimination() ADCE.cpp:0:0
 #7 0x00005590a3dae4a9 llvm::ADCEPass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0x37f94a9)
 #8 0x00005590a35590dd llvm::detail::PassModel<llvm::Function, llvm::ADCEPass, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0x2fa40dd)
 #9 0x00005590a2d2c314 llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0x2777314)
#10 0x00005590a11e95ed llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (../../main-github/llvm/build-all/bin/opt+0xc345ed)
#11 0x00005590a2d306fe llvm::ModuleToFunctionPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (../../main-github/llvm/build-all/bin/opt+0x277b6fe)
#12 0x00005590a11e938d llvm::detail::PassModel<llvm::Module, llvm::ModuleToFunctionPassAdaptor, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (../../main-github/llvm/build-all/bin/opt+0xc3438d)
#13 0x00005590a2d2b4a4 llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (../../main-github/llvm/build-all/bin/opt+0x27764a4)
#14 0x00005590a0e2314e llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::PassPlugin>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool) (../../main-github/llvm/build-all/bin/opt+0x86e14e)
#15 0x00005590a0e3181e main (../../main-github/llvm/build-all/bin/opt+0x87c81e)
#16 0x00007f3962dc0555 __libc_start_main (/lib64/libc.so.6+0x22555)
#17 0x00005590a0e1d8a0 _start (../../main-github/llvm/build-all/bin/opt+0x8688a0)
Segmentation fault (core dumped)

Ping @labrinea

Can you take a look at this regression?

It also crashes when using opt -passes="ipsccp,print<postdomtree>" -mtriple=hexagon bbi-82827.ll -o /dev/null.
So it is something with the PostDominatorTree analysis getting messed up?

@aeubanks / @asbirlea : Can you see any obvious mistake here, when it comes to new-PM and analysis passes?

PostDominatorTreeAnalysis is preserved (regardless of changes). There is a FAM.getCachedResult<PostDominatorTreeAnalysis>(F) that might happen if getAnalysis is called(?). But I think the new thing here is that when using BlockFrequencyAnalysis that will run the PostDominatorTreeAnalysis (via BranchProbabilityAnalysis) even if it isn't cached already.

A workaround(?) might be to comment out PA.preserve<PostDominatorTreeAnalysis>(). But I'm not sure it solve the problem completely (the given repoducer no longer crashes at least).

bjope added a comment.May 26 2023, 2:26 AM

Maybe this is what is needed:

diff --git a/llvm/lib/Transforms/IPO/SCCP.cpp b/llvm/lib/Transforms/IPO/SCCP.cpp
index 21b89ce4de40..e5a940f83311 100644
--- a/llvm/lib/Transforms/IPO/SCCP.cpp
+++ b/llvm/lib/Transforms/IPO/SCCP.cpp
@@ -394,9 +394,10 @@ PreservedAnalyses IPSCCPPass::run(Module &M, ModuleAnalysisManager &AM) {
   };
   auto getAnalysis = [&FAM](Function &F) -> AnalysisResultsForFn {
     DominatorTree &DT = FAM.getResult<DominatorTreeAnalysis>(F);
+    PostDominatorTree &PDT = FAM.getResult<PostDominatorTreeAnalysis>(F);
     return {
         std::make_unique<PredicateInfo>(F, DT, FAM.getResult<AssumptionAnalysis>(F)),
-        &DT, FAM.getCachedResult<PostDominatorTreeAnalysis>(F) };
+        &DT, &PDT };
   };
 
   if (!runIPSCCP(M, DL, &FAM, GetBFI, GetTLI, GetTTI, GetAC, getAnalysis,

By providing a mutable PDT, IPSCCP+solver would end up updating the PDT when doing changes?

Sorry, I didn't get any notification for this regression. I'll investigate.

bjope added a comment.EditedMay 26 2023, 8:43 AM

So to summarize what I think is going on (more or less by just looking at the code, I haven't been running it in a debugger):

  1. When setting up the DomTreeUpdater the use of FAM.getCachedResult<PostDominatorTreeAnalysis>(F) might result in not finding any cached PDT. So the DomTreeUpdater will not update PDT.
  2. Later as a side-effect of using BlockFrequencyAnalysis, a PDT is created and calculated.
  3. Optimizations are done, without updating the now existing PDT.
  4. The pass says that PDT is preserved. But it has not been updated.
bjope added a comment.May 26 2023, 8:49 AM

Beware: The reproducer provided by uabelho is reduced, but it is a bit ugly. I spent some time trying to clean it up, but even when using valgrind/asan it is hard to get a nice and small reproducer (e.g. that would show an incorrect PDT when using print<postdomtree>). I even ended up in a situation when renaming the labels used in the IR suddenly made the reproducer no longer crashing. So the reproducer seem to be a bit sensitive to how things end up in memory(?).

So to summarize what I think is going on (more or less by just looking at the code, I haven't been running it in a debugger):

  1. When setting up the DomTreeUpdater the use of FAM.getCachedResult<PostDominatorTreeAnalysis>(F) might result in not finding any cached PDT. So the DomTreeUpdater will not update PDT.
  2. Later as a side-effect of using BlockFrequencyAnalysis, a PDT is created and calculated.
  3. Optimizations are done, without updating the now existing PDT.
  4. The pass says that PDT is preserved. But it has not been updated.

sounds likely to me

writing a StandardInstrumentations instrumentation that checks if cached analyses like (Post)DomTree, MSAA, etc are still valid after a function pass would be cool and would probably catch this more easily (assuming PDT.verify() would complain here)

bjope added a comment.May 29 2023, 5:23 AM

No idea why phabricator thinks that D151648 is a reverting change. It is a proposed fix, not a revert.

llvm/test/Transforms/FunctionSpecialization/function-specialization-always-inline.ll