Fix division by zero in ColdBlockInfo::inferFromEdgeProfile

[karim-alweheshy]

Summary

This PR fixes a division by zero crash in the Swift compiler's profile-guided optimization implementation and provides user-configurable strategies for handling zero-count blocks, inspired by similar improvements in LLVM.

Background

LLVM recently addressed similar issues with zero-count blocks in profile-guided optimization (see LLVM PR #154437), which added configurable strategies for handling blocks that were instrumented but never executed during profiling. This change brings similar capabilities to Swift.

Problem

The issue manifests in ColdBlockInfo::inferFromEdgeProfile when calculating taken probabilities:

double takenProbability = succCount[i].getValue() / (double)totalCount.getValue();

When totalCount is 0 (block was instrumented but never executed), this causes a division by zero crash during SIL optimization passes.

Solution

Added a check for totalCount.getValue() < 1 before the division operation, with user-configurable strategies for handling zero-count blocks:

Command-Line Option

-sil-zero-count-strategy=<strategy>

Available Strategies

1. conservative (default): Skip inference, let other heuristics decide

   • Safe default behavior that prevents crashes
   • Makes no assumptions about missing execution data
   • Follows the same pattern as other Swift compiler components

2. optimistic: Assume zero-count blocks are cold

   • Logical assumption: zero execution counts = never executed = cold code
   • Useful for aggressive size optimization when profile data is trusted
   • Aligns with the reasoning that warm/important code would have execution counts

Changes

1. ColdBlockInfo.cpp:

   • Added zero-count check with configurable strategies
   • Command-line option with enum-based strategy selection
   • Debug logging for strategy decisions

2. cold_block_zero_count.sil:

   • Comprehensive test cases covering both strategies
   • Tests blocks with zero execution counts on all branches
   • Tests mixed zero/non-zero execution counts
   • Tests inlining scenarios with zero-count blocks

Design Rationale

The two-strategy approach is based on logical reasoning about zero-count scenarios:

• Zero counts indicate: either "truly not executed" or "missing profile data"
• Conservative approach: When uncertain, make no assumptions (safe default)
• Optimistic approach: Assume zero counts mean cold code (aggressive optimization)

No "aggressive/warm" strategy is included because warm/important code would logically have execution counts during profiling.

Testing

The test reproduces the division by zero condition and verifies both strategies handle it gracefully:

# Conservative strategy (default) %target-sil-opt %s -performance-inline # Optimistic strategy  %target-sil-opt %s -performance-inline -sil-zero-count-strategy=optimistic

Profile data with zero counts is common for error handling paths, platform-specific code, and rarely executed functions. This fix ensures the compiler handles such cases without crashing while providing optimization flexibility.

Compatibility

• Backward compatible: conservative strategy is the default
• Follows existing Swift patterns for command-line options
• Aligns with LLVM's approach to profile-guided optimization control

[kavon]

Thanks! Overall this looks great. Do you think you could squash this into one commit?

[kavon]

A ProfileCounter is effectively an Optional<UInt> and there's some nuance here between .none and .some(0). I believe .none was meant to convey "missing" data, perhaps due to sampling misses, and the .some(0) was meant to convey "definitely no execution happened".

So, I think some of this ZeroCountStrategy stuff needs to also be added a few lines up, to decide whether to interpret a .none value as 0. I think this interpretation should only happen in the optimistic strategy if there's at least one ProfileCounter in the block's successors list with a .some, regardless of what the count of it is. Otherwise the optimistic strategy may mark every block cold.

[karim-alweheshy]

TODO: Implement full SamplePGO profile count propagation algorithm

The current implementation handles missing profile data with a simple
optimistic strategy (treat missing as zero when evidence of profiling exists).
The SamplePGO paper by Diego Novillo describes more sophisticated techniques
for inferring missing counts through iterative propagation across the CFG:

1. Equivalence Classes: Compute sets of blocks guaranteed to execute the
   same number of times using dominance, post-dominance, and loop nesting:

   • B1 dominates B2
   • B2 post-dominates B1
   • B1 and B2 in same loop nest
     All blocks in same class get same weight.

2. Iterative Propagation: Use flow conservation to infer unknown weights:
   Weight(Block) = Sum(Incoming Edges) = Sum(Outgoing Edges)
   Algorithm:

   • If all edges known → compute block weight
   • If block known and one edge unknown → compute that edge
   • Iterate until convergence or max iterations

3. Multi-Successor Support: Extend beyond 2-successor limitation to handle
   switch statements, multi-way branches, and complex control flow.

Future implementation should:

• Add propagateProfileWeights() method as a third pass after this collection
• Use the existing 'counters' vector as input (already distinguishes missing)
• Maintain backward compatibility with ZeroCountStrategy flag
• Support incremental adoption (propagation as optional enhancement)
• Add new command-line flag: -sil-profile-propagation=[none|basic|full]

The paper reports achieving up to 98% of instrumentation-based PGO performance
gains with this approach, making it highly valuable for handling incomplete
sampling profiles in real-world scenarios.

Reference: Diego Novillo. "SamplePGO - The Power of Profile Guided
Optimizations without the Usability Burden."
LLVM-HPC 2014. DOI: 10.1109/LLVM-HPC.2014.8
PDF: https://storage.googleapis.com/gweb-research2023-media/pubtools/pdf/45290.pdf

[kavon]

TODO: Implement full SamplePGO profile count propagation algorithm

How about we keep this PR focused on the bug fix and save the new algorithm for a follow-up PR?

[kavon]

Please ping me when you think this is ready for another round of review! Thanks.

[kavon]

nit: std::optional<ProfileCounter> is redundant here. ProfileCounter already has a representation of "missing"

[kavon]

This is applying the optimistic strategy unconditionally. I would expect it to be aligned with the flag.

[kavon]

nit: inaccurate addition to this comment

There already is propagation. The reason for this limitation is hinted at in the original comment: there's only two temperatures being set and propagated for blocks. We set a block cold only if it's taken with very low probability, currently under 3%, otherwise it's just "warm". If a block has 100 successors and they are all taken with equal probability of 1%, then we'd mistakenly mark all blocks cold.

Definitely could use a better strategy here, but I think for this PR let's stay focused on fixing the divide-by-zero and having the "assume cold for missing data".

[karim-alweheshy]

we consider missing data to be cold only when data is partial i.e. on of the branches was executed at least once

[kavon]

The only CHECK-* lines in this test are to ensure the existence of the function after the performance inliner runs. There's nothing verifying that blocks got marked cold or not with the different zero-count strategies. As an example of how to do that, see test/SILOptimizer/cold_block_info.swift where I use the debug output from this analysis check that it's working as expected.