https://github.com/maksfb approved this pull request.
LGTM with the nit addressed.
https://github.com/llvm/llvm-project/pull/93759
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
@@ -596,6 +597,9 @@ class RewriteInstance {
NameResolver NR;
+ // Regex object matching split function names.
+ const Regex ColdFragment{"(.*)\\.(cold|warm)(\\.[0-9]+)?"};
maksfb wrote:
nit: s/ColdFragment/FunctionFragmentTemplate/
https://github.com/maksfb edited https://github.com/llvm/llvm-project/pull/93759
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
https://github.com/maksfb approved this pull request.
LGTM. Thanks.
https://github.com/llvm/llvm-project/pull/92713
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
@@ -4788,13 +4788,25 @@ void RewriteInstance::updateELFSymbolTable(
if (!IsDynSym && shouldStrip(Symbol))
continue;
+Expected SymbolName = Symbol.getName(StringSection);
maksfb wrote:
Can we move the code that checks for special symbols here
@@ -2386,25 +2362,26 @@ std::error_code
DataAggregator::writeBATYAML(BinaryContext ,
return std::pair(BlockIt->first, BlockIt->second.getBBIndex());
};
- for (const auto &[FromOffset, SuccKV] : Branches.IntraIndex) {
-const auto &[_, Index] =
https://github.com/maksfb edited https://github.com/llvm/llvm-project/pull/91289
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
https://github.com/maksfb approved this pull request.
Looks good on my end.
https://github.com/llvm/llvm-project/pull/91289
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://github.com/maksfb approved this pull request.
Please see the nit for the error message (caps and new line). Otherwise LGTM.
https://github.com/llvm/llvm-project/pull/91273
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
@@ -2378,21 +2379,27 @@ std::error_code
DataAggregator::writeBATYAML(BinaryContext ,
return CSI;
};
+ // Lookup containing basic block offset and index
+ auto getBlock = [](uint32_t Offset) {
+auto BlockIt = BlockMap.upper_bound(Offset);
+
https://github.com/maksfb edited https://github.com/llvm/llvm-project/pull/91273
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
@@ -2378,21 +2378,24 @@ std::error_code
DataAggregator::writeBATYAML(BinaryContext ,
return CSI;
};
+ // Lookup containing basic block offset and index
+ auto getBlock = [](uint32_t Offset) {
+auto BlockIt = BlockMap.upper_bound(Offset);
+
@@ -2378,21 +2378,24 @@ std::error_code
DataAggregator::writeBATYAML(BinaryContext ,
return CSI;
};
+ // Lookup containing basic block offset and index
+ auto getBlock = [](uint32_t Offset) {
+auto BlockIt = BlockMap.upper_bound(Offset);
+
https://github.com/maksfb edited https://github.com/llvm/llvm-project/pull/90807
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
@@ -1167,6 +1167,21 @@ void BinaryFunction::handleAArch64IndirectCall(MCInst
,
}
}
+std::optional
+BinaryFunction::disassembleInstructionAtOffset(uint64_t Offset) const {
+ assert(CurrentState == State::Empty);
+ assert(Offset < MaxSize && "invalid offset");
@@ -1167,6 +1167,21 @@ void BinaryFunction::handleAArch64IndirectCall(MCInst
,
}
}
+std::optional
+BinaryFunction::disassembleInstructionAtOffset(uint64_t Offset) const {
+ assert(CurrentState == State::Empty);
+ assert(Offset < MaxSize && "invalid offset");
+ ErrorOr>
@@ -1167,6 +1167,21 @@ void BinaryFunction::handleAArch64IndirectCall(MCInst
,
}
}
+std::optional
+BinaryFunction::disassembleInstructionAtOffset(uint64_t Offset) const {
+ assert(CurrentState == State::Empty);
maksfb wrote:
nit: add message.
https://github.com/maksfb approved this pull request.
Please address the nits. Otherwise - good to go.
https://github.com/llvm/llvm-project/pull/90807
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
@@ -492,6 +486,10 @@ uint64_t BoltAddressTranslation::translate(uint64_t
FuncAddress,
const uint32_t Val = KeyVal->second >> 1; // dropping BRANCHENTRY bit
if (IsBranchSrc) {
+// Branch entry is found in BAT
+if (KeyVal->first == Offset && KeyVal->second &
@@ -48,10 +48,9 @@ static cl::opt InputFilename(cl::Positional,
cl::Required,
cl::cat(BatDumpCategory));
-static cl::list Translate("translate",
-
@@ -24,6 +24,32 @@ READ-BAT-CHECK-NOT: BOLT-ERROR: unable to save profile in
YAML format for input
READ-BAT-CHECK: BOLT-INFO: Parsed 5 BAT entries
READ-BAT-CHECK: PERF2BOLT: read 79 aggregated LBR entries
+# Check handling of a branch not in BAT (added by BOLT)
+RUN:
@@ -478,18 +478,34 @@ uint64_t BoltAddressTranslation::translate(uint64_t
FuncAddress,
return Offset;
const MapTy = Iter->second;
+ if (IsBranchSrc) {
+// Try exact lookup first
+auto KeyVal = Map.find(Offset);
+if (KeyVal != Map.end() && KeyVal->second &
https://github.com/maksfb approved this pull request.
https://github.com/llvm/llvm-project/pull/90424
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
https://github.com/maksfb approved this pull request.
https://github.com/llvm/llvm-project/pull/87853
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
https://github.com/maksfb approved this pull request.
https://github.com/llvm/llvm-project/pull/87569
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
https://github.com/maksfb approved this pull request.
https://github.com/llvm/llvm-project/pull/87743
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
@@ -2341,86 +2341,62 @@ std::error_code
DataAggregator::writeBATYAML(BinaryContext ,
YamlBF.NumBasicBlocks = BAT->getNumBasicBlocks(FuncAddress);
const BoltAddressTranslation::BBHashMapTy =
BAT->getBBHashMap(FuncAddress);
+
@@ -2341,86 +2341,62 @@ std::error_code
DataAggregator::writeBATYAML(BinaryContext ,
YamlBF.NumBasicBlocks = BAT->getNumBasicBlocks(FuncAddress);
const BoltAddressTranslation::BBHashMapTy =
BAT->getBBHashMap(FuncAddress);
+
@@ -38,10 +38,63 @@
# RUN: llvm-bolt %t.exe -o %t.bat --data %t.fdata --funcs=func \
# RUN: --split-functions --split-strategy=all --split-all-cold --enable-bat
+# Prepare pre-aggregated profile using %t.bat
maksfb wrote:
Use double `#`s.
@@ -59,10 +59,29 @@
# RUN: llvm-bolt %t.exe -o %t.bat2 --data %t.fdata --funcs=main,func \
# RUN: --split-functions --split-strategy=all --split-all-cold --enable-bat
+# Prepare pre-aggregated profile using %t.bat
maksfb wrote:
Nit: use double `#`s.
@@ -245,14 +245,12 @@ class DataAggregator : public DataReader {
/// disassembled BinaryFunctions
BinaryFunction *getBinaryFunctionContainingAddress(uint64_t Address) const;
+ /// Perform BAT translation for a given \p Func and return the parent
+ /// BinaryFunction or
https://github.com/maksfb approved this pull request.
https://github.com/llvm/llvm-project/pull/87967
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
maksfb wrote:
Okay. That explains the increase.
https://github.com/llvm/llvm-project/pull/87967
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
maksfb wrote:
> > Creating a proper FDE entry is the right thing to do regardless of BAT. How
> > large is the regression?
>
> The largest I've seen is 34M->39M (HHVM instrumentation).
Did you check if we patch every single function in he original `.text`? How
many functions are there?
maksfb wrote:
> > Let's make `hasCFI()` return true for injected functions.
>
> Given that this change increases the size of eh_frame section, should we make
> it dependent on `enable-bat`, i.e. when we expect to feed the binary back to
> BOLT?
>
> Because otherwise this "fix" is a pure size
https://github.com/maksfb commented:
Is information we emit in the symbol table insufficient to establish the
fragment relationship?
https://github.com/llvm/llvm-project/pull/87968
___
llvm-branch-commits mailing list
https://github.com/maksfb commented:
Let's make `hasCFI()` return true for injected functions.
https://github.com/llvm/llvm-project/pull/87967
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://github.com/maksfb edited https://github.com/llvm/llvm-project/pull/86219
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
@@ -161,6 +164,10 @@ class BoltAddressTranslation {
/// Map a function to its secondary entry points vector
std::unordered_map> SecondaryEntryPointsMap;
+ /// Translates a given \p Symbol into a BinaryFunction and
+ /// Returns a secondary entry point id for a given \p
@@ -123,6 +124,12 @@ class BoltAddressTranslation {
std::unordered_map>
getBFBranches(uint64_t FuncOutputAddress) const;
+ /// For a given \p Symbol in the output binary, returns a corresponding pair
+ /// of parent BinaryFunction and secondary entry point in it.
https://github.com/maksfb approved this pull request.
Looks good with nits addressed.
https://github.com/llvm/llvm-project/pull/86219
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
@@ -27,25 +28,55 @@ namespace bolt {
/// Set CallSiteInfo destination fields from \p Symbol and return a target
/// BinaryFunction for that symbol.
-static const BinaryFunction *setCSIDestination(const BinaryContext ,
-
https://github.com/maksfb approved this pull request.
https://github.com/llvm/llvm-project/pull/87123
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
@@ -197,8 +197,10 @@ void BoltAddressTranslation::writeMaps(std::map ,
? SecondaryEntryPointsMap[Address].size()
: 0;
if (Cold) {
- size_t HotIndex =
- std::distance(ColdPartSource.begin(), ColdPartSource.find(Address));
+ //
@@ -27,25 +28,55 @@ namespace bolt {
/// Set CallSiteInfo destination fields from \p Symbol and return a target
/// BinaryFunction for that symbol.
-static const BinaryFunction *setCSIDestination(const BinaryContext ,
-
@@ -27,25 +28,55 @@ namespace bolt {
/// Set CallSiteInfo destination fields from \p Symbol and return a target
/// BinaryFunction for that symbol.
-static const BinaryFunction *setCSIDestination(const BinaryContext ,
-
@@ -27,25 +28,55 @@ namespace bolt {
/// Set CallSiteInfo destination fields from \p Symbol and return a target
/// BinaryFunction for that symbol.
-static const BinaryFunction *setCSIDestination(const BinaryContext ,
-
@@ -27,25 +28,55 @@ namespace bolt {
/// Set CallSiteInfo destination fields from \p Symbol and return a target
/// BinaryFunction for that symbol.
-static const BinaryFunction *setCSIDestination(const BinaryContext ,
-
@@ -123,6 +123,9 @@ class BoltAddressTranslation {
std::unordered_map>
getBFBranches(uint64_t FuncOutputAddress) const;
+ /// Returns a secondary entry point id for a given function and offset.
maksfb wrote:
"... function at a given \p Address..."
https://github.com/maksfb approved this pull request.
LGTM
https://github.com/llvm/llvm-project/pull/82128
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
@@ -78,32 +97,15 @@ YAMLProfileWriter::convert(const BinaryFunction , bool
UseDFS) {
if (!ICSP)
continue;
for (const IndirectCallProfile : ICSP.get()) {
- StringRef TargetName = "";
- CSI.DestId = 0; // designated for unknown
51 matches
Mail list logo
