[PATCH] D13914: Tolerate negative offset when matching sample profile.

[Diego Novillo via llvm-commits]

dnovillo accepted this revision.
dnovillo added a comment.
This revision is now accepted and ready to land.

LGTM with the additional check.  Thanks.


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:186
@@ +185,3 @@
+unsigned SampleProfileLoader::getOffset(unsigned L, unsigned H) const {
+  return (L - H) & 0xffff;
+}
----------------
I would now assert that this is the case (ie, L - H fits in 16 bits).  It's true that most input will come straight out of create_llvm_prof, but not all of it will.  In particular, the offsets in the input profiles are specified to be 32-bit unsigned integers.

Alternatively, we could assert this in ProfileData/SampleProfReader.cpp itself.  This needs to be done in all 3 readers.  Perhaps that's a better place to assert this.

================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:245-246
@@ -234,4 +244,4 @@
 
-  ErrorOr<uint64_t> R =
-      FS->findSamplesAt(Lineno - HeaderLineno, DIL->getDiscriminator());
+  ErrorOr<uint64_t> R = FS->findSamplesAt(getOffset(Lineno, HeaderLineno),
+                                          DIL->getDiscriminator());
   if (R)
----------------
danielcdh wrote:
> Done.
> 
> In create_llvm_prof, offset is only encoded in 16 bits.
Sure, but we're sufficiently removed from create_llvm_prof here that it makes sense to remind the user :)


http://reviews.llvm.org/D13914