This is an archive of the discontinued LLVM Phabricator instance.

[PseudoProbe] Only emit discriminstor in FS-AFDO mode.
ClosedPublic

Authored by hoy on May 15 2023, 4:34 PM.

Details

Summary

Despite previous effort D148569: [PseudoProbe] Clean up dwarf discriminator and avoid duplicating factor. to avoid screwing up existing disrminator field, I'm still seeing some call probes getting a non-zero discriminator eventually in non-FS mode. It could be related to callsite merge. While they are investigated I'm disabling discriminator emission for non-FS mode. This avoids breaking the compatiblity with older tools like llvm-profgen and bolt.

Diff Detail

Event Timeline

hoy created this revision.May 15 2023, 4:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 15 2023, 4:34 PM
hoy requested review of this revision.May 15 2023, 4:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 15 2023, 4:34 PM
hoy updated this revision to Diff 522665.May 16 2023, 9:28 AM

Switching to checking block probe for discriminator emission.

wenlei accepted this revision.May 16 2023, 5:33 PM

thanks for the fix, lgtm.

llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
49

should we put in an assert to make sure we catch lurking non-zero discriminators? i.e. when FS is off, probe should always have zero discriminator?

This revision is now accepted and ready to land.May 16 2023, 5:33 PM
hoy added inline comments.May 16 2023, 5:43 PM
llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
49

Good point. Actually there is already an assert on IR level https://github.com/llvm/llvm-project/blob/b7d9322b4963e620dfd12246816e6f7b2da5fd88/llvm/lib/IR/PseudoProbe.cpp#L67, let me also add an assert here for MIR.

hoy updated this revision to Diff 522855.May 16 2023, 5:43 PM

Adding assert.

This revision was landed with ongoing or failed builds.May 16 2023, 9:35 PM
This revision was automatically updated to reflect the committed changes.