Page MenuHomePhabricator

[PGO/SamplePGO][NFC] Move the function updateProfWeight from Instruction to CallInst
ClosedPublic

Authored by wmi on Apr 19 2019, 11:55 AM.

Details

Summary

The issue was raised here: https://reviews.llvm.org/D60903#1472783

The function Instruction::updateProfWeight is only used for CallInst in profile update. From the current interface, it is very easy to think that the function can also be used for branch instruction. However, Branch instruction does't need the scaling the function provides for branch_weights and VP (value profile), in addition, scaling may introduce inaccuracy for branch probablity.

The patch moves the function updateProfWeight from Instruction class to CallInst to remove the confusion. The patch also changes the scaling of branch_weights from a loop to a block because we know that ProfileData for branch_weights of CallInst will only have two operands at most.

Diff Detail

Repository
rL LLVM

Event Timeline

wmi created this revision.Apr 19 2019, 11:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 19 2019, 11:55 AM
davidxl accepted this revision.Apr 19 2019, 12:19 PM

This is not completely NFC as it modifies branch weight update behavior (though correctly), so please do more pgo testings to avoid surprises (e.g, some weird dependencies on old behavior).

This revision is now accepted and ready to land.Apr 19 2019, 12:19 PM
wmi added a comment.Apr 22 2019, 8:30 AM

Run internal FDO testing and the speedup is within expected range.

This revision was automatically updated to reflect the committed changes.

Can you please say what is the reason to move the function to CallInst only but not for CallBase?

Can you please say what is the reason to move the function to CallInst only but not for CallBase?

I'm sorry, I get it.