[BOLT] Fix the inaccurate profile data check #136278
Conversation
WangJee
requested review from
aaupov,
ayermolo,
dcci,
maksfb,
rafaelauler and
yota9
as code owners
|
@llvm/pr-subscribers-bolt Author: wangjue (WangJee) ChangesThe error is caused by the fact that when acquiring the profile data, the instruction at offset is PseudoCALL, but when performing profile verification, PseudoCALL is converted to AUIPC and JALR instructions, and the offset obtained is JALR; therefore, the profile data is considered invalid. Full diff: https://github.com/llvm/llvm-project/pull/136278.diff 1 Files Affected:
diff --git a/bolt/lib/Profile/DataReader.cpp b/bolt/lib/Profile/DataReader.cpp
index f2e999bbfdc6d..258a36c86afb2 100644
--- a/bolt/lib/Profile/DataReader.cpp
+++ b/bolt/lib/Profile/DataReader.cpp
@@ -524,6 +524,9 @@ float DataReader::evaluateProfileData(BinaryFunction &BF,
// when we identify tail calls, so they are still represented
// by regular branch instructions and we need isBranch() here.
MCInst *Instr = BF.getInstructionAtOffset(BI.From.Offset);
+ // If t's a RISCV PseudoCALL - fix it
+ if (!Instr && BC.isRISCV())
+ Instr = BF.getInstructionAtOffset(BI.From.Offset + 4);
// If it's a prefix - skip it.
if (Instr && BC.MIB->isPrefix(*Instr))
Instr = BF.getInstructionAtOffset(BI.From.Offset + 1);
|
|
solve the issue #136276 |
WangJee
changed the title
|
Thank you for investigating the issue. I see that we have FixRISCVCallsPass that runs before instrumentation and replaces all calls with PseudoCALL instructions for JITLink relaxation. But the pass may replace either jalr or auipc+jalr pair with a pseudocall, so in the general case it's not correct to always adjust an offset by 4. |
bolt/lib/Profile/DataReader.cpp
Outdated
| // If it's a RISCV PseudoCALL - fix it | ||
| if (!Instr && BC.isRISCV()) |
There was a problem hiding this comment.
Use isRISCVCall(const MCInst &First, const MCInst &Second)
There was a problem hiding this comment.
MCInst *Instr = BF.getInstructionAtOffset(BI.From.Offset);
But if this method is used, the first instruction will return nullptr, and the second instruction can correctly obtain jalr
There was a problem hiding this comment.
@aaupov Could you please review the new changes?
Thank you for your suggestion, but if it is jalr's scenario, Instr will not be nullptr? |
3 participants
The error is caused by the fact that when acquiring the profile data, the instruction at offset is PseudoCALL, but when performing profile verification, PseudoCALL is converted to AUIPC and JALR instructions, and the offset obtained is JALR; therefore, the profile data is considered invalid.