Repository navigation
[BFI] Scale block frequencies with native 128-bit integers (NFC) - #239953
Conversation
|
@llvm/pr-subscribers-llvm-analysis Author: Florian Hahn (fhahn) ChangesUpdate setBlockFreqAndScale to use native 128 bit integers instead of APInt if available, as this improves compile-time: stage1-O3: -0.7% Assisted-by: Opus 5.5 Full diff: https://github.com/llvm/llvm-project/pull/239953.diff 1 Files Affected:
diff --git a/llvm/lib/Analysis/BlockFrequencyInfo.cpp b/llvm/lib/Analysis/BlockFrequencyInfo.cpp
index efb4f0d467b94..dcdb221587757 100644
--- a/llvm/lib/Analysis/BlockFrequencyInfo.cpp
+++ b/llvm/lib/Analysis/BlockFrequencyInfo.cpp
@@ -189,24 +189,34 @@ void BlockFrequencyInfo::setBlockFreq(const BasicBlock *BB,
BFI->setBlockFreq(BB, Freq);
}
+/// Return min(BBFreq * NewFreq / OldFreq, UINT64_MAX). Multiplies before
+/// dividing, to minimize loss of precision.
+static uint64_t scaleFrequency(uint64_t BBFreq, uint64_t NewFreq,
+ uint64_t OldFreq) {
+ assert(OldFreq != 0 && "Divide by zero?");
+ // Unsigned division is an expensive operation in the general case. If this
+ // ends up being a hot spot, one of the options proposed in
+ // https://reviews.llvm.org/D28535#650071 could be used to avoid this.
+ // clang-cl defines __SIZEOF_INT128__, but MSVC's runtime does not provide
+ // the 128-bit division helper (__udivti3).
+#if defined(__SIZEOF_INT128__) && !defined(_MSC_VER)
+ __uint128_t Scaled = __uint128_t(BBFreq) * NewFreq / OldFreq;
+ return std::min<__uint128_t>(Scaled, UINT64_MAX);
+#else
+ return (APInt(128, BBFreq) * NewFreq).udiv(OldFreq).getLimitedValue();
+#endif
+}
+
void BlockFrequencyInfo::setBlockFreqAndScale(
const BasicBlock *ReferenceBB, BlockFrequency Freq,
SmallPtrSetImpl<BasicBlock *> &BlocksToScale) {
assert(BFI && "Expected analysis to be available");
- // Use 128 bits APInt to avoid overflow.
- APInt NewFreq(128, Freq.getFrequency());
- APInt OldFreq(128, BFI->getBlockFreq(ReferenceBB).getFrequency());
- APInt BBFreq(128, 0);
+ uint64_t NewFreq = Freq.getFrequency();
+ uint64_t OldFreq = BFI->getBlockFreq(ReferenceBB).getFrequency();
for (auto *BB : BlocksToScale) {
- BBFreq = BFI->getBlockFreq(BB).getFrequency();
- // Multiply first by NewFreq and then divide by OldFreq
- // to minimize loss of precision.
- BBFreq *= NewFreq;
- // udiv is an expensive operation in the general case. If this ends up being
- // a hot spot, one of the options proposed in
- // https://reviews.llvm.org/D28535#650071 could be used to avoid this.
- BBFreq = BBFreq.udiv(OldFreq);
- BFI->setBlockFreq(BB, BlockFrequency(BBFreq.getLimitedValue()));
+ uint64_t BBFreq = BFI->getBlockFreq(BB).getFrequency();
+ BFI->setBlockFreq(BB,
+ BlockFrequency(scaleFrequency(BBFreq, NewFreq, OldFreq)));
}
BFI->setBlockFreq(ReferenceBB, Freq);
}
|
aengelke
left a comment
There was a problem hiding this comment.
LGTM. This bothered me since quite a while, but I never got around to fix it. Thanks!
MaskRay
left a comment
There was a problem hiding this comment.
LGTM. The only caller is in InlineFunction.cpp, where cloned block frequencies need rescaling.
MaskRay
left a comment
There was a problem hiding this comment.
Drop https://reviews.llvm.org/D28535#650071 reference
| // Unsigned division is an expensive operation in the general case. If this | ||
| // ends up being a hot spot, one of the options proposed in | ||
| // https://reviews.llvm.org/D28535#650071 could be used to avoid this. |
There was a problem hiding this comment.
The D28535 note can go: with __uint128_t the remaining divq is ~6 of ~44 instructions per scaled block in a microbenchmark, and the options in that thread trade away precision. FWIW the Windows release (clang-cl, _MSC_VER) still takes the APInt path.
| // Unsigned division is an expensive operation in the general case. If this | |
| // ends up being a hot spot, one of the options proposed in | |
| // https://reviews.llvm.org/D28535#650071 could be used to avoid this. |
Update setBlockFreqAndScale to use native 128 bit integers instead of APInt if available, as this improves compile-time: stage1-O3: -0.7% stage1-ReleaseThinLTO: -0.08% stage1-ReleaseLTO-g: -0.06% stage1-aarch64-O3: -0.07% stage2-O3: -0.07% stage2-clang: -0.14% https://llvm-compile-time-tracker.com/compare.php?from=f82380b4c3ff92a6293f03f7c02e40249ba10e13&to=b885f69205b43f51764127d753eaed4fd91a12e8&stat=instructions:u Assisted-by: Opus 5.5
49c9a52 to
efbb3f8
Compare
… (NFC) (#239953) Update setBlockFreqAndScale to use native 128 bit integers instead of APInt if available, as this improves compile-time: stage1-O3: -0.7% stage1-ReleaseThinLTO: -0.08% stage1-ReleaseLTO-g: -0.06% stage1-aarch64-O3: -0.07% stage2-O3: -0.07% stage2-clang: -0.14% https://llvm-compile-time-tracker.com/compare.php?from=f82380b4c3ff92a6293f03f7c02e40249ba10e13&to=b885f69205b43f51764127d753eaed4fd91a12e8&stat=instructions:u Assisted-by: Opus 5.5 PR: llvm/llvm-project#239953
… (NFC) (#239953) Update setBlockFreqAndScale to use native 128 bit integers instead of APInt if available, as this improves compile-time: stage1-O3: -0.7% stage1-ReleaseThinLTO: -0.08% stage1-ReleaseLTO-g: -0.06% stage1-aarch64-O3: -0.07% stage2-O3: -0.07% stage2-clang: -0.14% https://llvm-compile-time-tracker.com/compare.php?from=f82380b4c3ff92a6293f03f7c02e40249ba10e13&to=b885f69205b43f51764127d753eaed4fd91a12e8&stat=instructions:u Assisted-by: Opus 5.5 PR: llvm/llvm-project#239953
Update setBlockFreqAndScale to use native 128 bit integers instead of APInt if available, as this improves compile-time:
stage1-O3: -0.7%
stage1-ReleaseThinLTO: -0.08%
stage1-ReleaseLTO-g: -0.06%
stage1-aarch64-O3: -0.07%
stage2-O3: -0.07%
stage2-clang: -0.14%
https://llvm-compile-time-tracker.com/compare.php?from=f82380b4c3ff92a6293f03f7c02e40249ba10e13&to=b885f69205b43f51764127d753eaed4fd91a12e8&stat=instructions:u
Assisted-by: Opus 5.5