[PATCH] D124481: [llvm][misexpect] Add tests for sample profiling

[Paul Kirth via Phabricator via llvm-commits]

paulkirth created this revision.
Herald added subscribers: ormris, wenlei, hiraditya.
Herald added a project: All.
paulkirth updated this revision to Diff 431527.
paulkirth added a comment.
paulkirth added a reviewer: tejohnson.
paulkirth published this revision for review.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

use smaller test-case for crashing input.


paulkirth added a comment.

As a general note, the test here will fail by design with the assertion in MisExpect.cpp re-enabled. That is the case I'm trying to fix/understand and address. We should probably still land the test, but the assertion will have to be removed unless we figure out a way to address the underlying issues.


paulkirth added a comment.

@tejohnson I have a test case that encapsulates when SampleProfiling crashes we saw w/ misexpect a while ago.

What I've been trying to figure out is how a profile ever gets zero weights like this.

My understanding of the PGO pipeline is that there are 3 modes where profiling weights are added: Front End Instrumentation inserted by clang, IR profiles, and SampleProfiling. My understanding was also that these modes were incompatible with one another, so you wouldn't have weights added by multiple sources, e.g., IR //and// Sample Profiling.

Given that model, I designed MissExpect to infer based on the profiling mode, if an existing weight should be interpreted as added by an `llvm.expect` intrinsic or not. Both sample profiles, and IR profiles occur after `llvm.expect` intrinsics are lowered, so they assume any existing weights are from annotations. When using Clang based instrumentation we assume that all existing weights come from profiles and we instead do the check when `llvm.expect` intrinsics are lowered. Given that model, I'm not sure how a program ever gets a set of branch weights that are all zeros.

Our checks for MisExpect in Sample profiles examine the branch weights on an instruction just prior to the profiling weight being added. There shouldn't be an opportunity for branch weights to be added to the instruction prior to this point, except from intrinsic lowering. We know that expect intrinsics won't do that, so we can safely ignore that possibility.

The only other scenario I can think of is that profiles are being mixed (e.g. Sample + IR) or that old bitcode with existing weights is being compiled again. If either case is legal then I we may need to consider a new design that can handle those scenarios.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124481

Files:
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/SampleProfile/Inputs/misexpect.prof
  llvm/test/Transforms/SampleProfile/misexpect-zero.ll
  llvm/test/Transforms/SampleProfile/misexpect.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124481.431527.patch
Type: text/x-patch
Size: 19296 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220524/748541d2/attachment.bin>