[PATCH] D153771: [BOLT][Instrumentation] Fix hash table memory corruption and append-pid option

[Rafael Auler via Phabricator via llvm-commits]

rafauler added inline comments.


================
Comment at: bolt/runtime/common.h:85
 
+#define PROT_READ 0x1  /* Page can be read.  */
+#define PROT_WRITE 0x2 /* Page can be written.  */
----------------
Amir wrote:
> @rafauler: so why don't we include mman.h? 
> 
> @treapster: since this particular change is NFC, can you please split it out to reduce functional changes surface and simplify testing and reviewing?
> 
> > [The patch should] be an isolated change. Independent changes should be submitted as separate patches as this makes reviewing easier.
> 
> https://llvm.org/docs/Contributing.html#how-to-submit-a-patch
If we include any headers, it will pull the file from the host system, which might not match the target system.

I actually prefer this particular patch as is (non-split). In general I think it's harder for me to work reviewing large stacks (unless there are lots of lines of code changed) and I will internally squash a stack of diffs into one for easier testing.

But I agree there are many benefits to splitting a diff, so it's fine to me if @treapster wants to do that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153771/new/

https://reviews.llvm.org/D153771