[PATCH] D117256: [memprof] Introduce a wrapper around MemInfoBlock.

Wed Jan 19 13:57:23 PST 2022

snehasish added a comment.

In D117256#3246296 <https://reviews.llvm.org/D117256#3246296>, @tejohnson wrote:

> lgtm but I think it would be nice to split into 2 patches, i.e. first the rename, which unfortunately does cause a little bit of extra churn on your end. But then it makes the new wrapper and inc file changes a little cleaner.

Done, uploaded the rename patch as D117722 <https://reviews.llvm.org/D117722>.

> by  MemProfData.inc, do we mean 'compiler-rt/include/profile/MemProfData.inc' the current file, or 'include/llvm/ProfileData/MemProfData.inc'?

@hiraditya Both files since they should be kept in sync. Instead of having dependencies across llvm and compiler-rt we use a .inc file (similar to InstrProfData.inc).

PTAL thanks!

================
Comment at: compiler-rt/include/profile/MemProfData.inc:86
+// Packed struct definition for MSVC. We can't use the PACKED macro defined in
+// MemProfData.inc since it would mean we are embedding a directive (the
+// #include for MIBEntryDef) into the macros which is undefined behaviour.
----------------
hiraditya wrote:
> by  MemProfData.inc, do we mean 'compiler-rt/include/profile/MemProfData.inc' the current file, or 'include/llvm/ProfileData/MemProfData.inc'?
> by  MemProfData.inc, do we mean 'compiler-rt/include/profile/MemProfData.inc' the current file, or 'include/llvm/ProfileData/MemProfData.inc'?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117256