This is an archive of the discontinued LLVM Phabricator instance.

[clang][CodeGenPGO] Don't use an invalid index when region counts disagree
ClosedPublic

Authored by lanza on Apr 28 2023, 6:11 PM.

Details

Summary

If we're using an old instrprof profile and the user passes
-Wno-profile-instr-out-of-date then we can get Decls with children
decl counts not matching the what the profile was written against. In
a particular case I was debugging we have 24 decls in the AST and 22
decls in the profile.

There probably isn't a "right" thing to do other than override the users
wish to force using the profiles, but that't the point of that flag in
the first place.

Diff Detail

Event Timeline

lanza created this revision.Apr 28 2023, 6:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2023, 6:11 PM
lanza requested review of this revision.Apr 28 2023, 6:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2023, 6:11 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
lanza added a comment.Apr 28 2023, 6:21 PM

Actually, this still is consumed even without that flag being passed. Anybody know the rational for not having at least an llvm major version check here?

smeenai accepted this revision.May 8 2023, 3:25 PM

Seems pretty reasonable to add a bounds check here. You should probably add a comment explaining the reasoning though.

This revision is now accepted and ready to land.May 8 2023, 3:25 PM
This revision was landed with ongoing or failed builds.May 10 2023, 8:00 PM
This revision was automatically updated to reflect the committed changes.