resolve comments

Author: mingmingl-llvm

llvm/include/llvm/ProfileData/MemProfYAML.h

@@ -264,7 +264,10 @@ template <> struct MappingTraits<memprof::YamlDataAccessProfData> {
 template <> struct MappingTraits<memprof::AllMemProfData> {
   static void mapping(IO &Io, memprof::AllMemProfData &Data) {
     Io.mapRequired("HeapProfileRecords", Data.HeapProfileRecords);
-    Io.mapOptional("DataAccessProfiles", Data.YamlifiedDataAccessProfiles);
+    // Map data access profiles if reading input, or if writing output &&
+    // the struct is populated.
+    if (!Io.outputting() || !Data.YamlifiedDataAccessProfiles.isEmpty())
+      Io.mapOptional("DataAccessProfiles", Data.YamlifiedDataAccessProfiles);
   }
 };
 

llvm/lib/ProfileData/IndexedMemProfData.cpp

@@ -228,7 +228,7 @@ static Error writeMemProfRadixTreeBased(
   OS.write(0ULL); // Reserve space for the memprof call stack payload offset.
   OS.write(0ULL); // Reserve space for the memprof record payload offset.
   OS.write(0ULL); // Reserve space for the memprof record table offset.
-  if (Version == memprof::Version4)
+  if (Version >= memprof::Version4)
     OS.write(0ULL); // Reserve space for the data access profile offset.
 
   auto Schema = memprof::getHotColdSchema();
@@ -258,6 +258,8 @@ static Error writeMemProfRadixTreeBased(
 
   uint64_t DataAccessProfOffset = 0;
   if (DataAccessProfileData.has_value()) {
+    assert(Version >= memprof::Version4 &&
+           "Data access profiles are added starting from v4");
     DataAccessProfOffset = OS.tell();
     if (Error E = (*DataAccessProfileData).get().serialize(OS))
       return E;
@@ -274,7 +276,7 @@ static Error writeMemProfRadixTreeBased(
       RecordPayloadOffset,
       RecordTableOffset,
   };
-  if (Version == memprof::Version4)
+  if (Version >= memprof::Version4)
     Header.push_back(DataAccessProfOffset);
   OS.patch({{HeaderUpdatePos, Header}});
 

llvm/test/tools/llvm-profdata/memprof-yaml.test

@@ -1,10 +1,17 @@
 ; RUN: split-file %s %t
 ; COM: The text format only supports the latest version.
+
+; Verify that the YAML output is identical to the YAML input.
+; memprof-in.yaml has both heap profile records and data access profiles.
 ; RUN: llvm-profdata merge --memprof-version=4 %t/memprof-in.yaml -o %t/memprof-out.indexed
 ; RUN: llvm-profdata show --memory %t/memprof-out.indexed > %t/memprof-out.yaml
 ; RUN: diff -b %t/memprof-in.yaml %t/memprof-out.yaml
 
-; Verify that the YAML output is identical to the YAML input.
+; memprof-in-no-dap has empty data access profiles.
+; RUN: llvm-profdata merge --memprof-version=4 %t/memprof-in-no-dap.yaml -o %t/memprof-out.indexed
+; RUN: llvm-profdata show --memory %t/memprof-out.indexed > %t/memprof-out-no-dap.yaml
+; RUN: diff -b %t/memprof-in-no-dap.yaml %t/memprof-out-no-dap.yaml
+
 ;--- memprof-in.yaml
 ---
 HeapProfileRecords:
@@ -38,12 +45,48 @@ HeapProfileRecords:
 DataAccessProfiles:
   SampledRecords:
     - Symbol:          abcde
+      Locations:
+      - FileName:      file2.h
+        Line:          123
+      - FileName:      file3.cpp
+        Line:          456
     - Hash:            101010
       Locations:
-        - FileName:        file
+        - FileName:        file.cpp
           Line:            233
   KnownColdSymbols:
     - foo
     - bar
   KnownColdHashes: [ 999, 1001 ]
 ...
+;--- memprof-in-no-dap.yaml
+---
+HeapProfileRecords:
+  - GUID:            0xdeadbeef12345678
+    AllocSites:
+      - Callstack:
+          - { Function: 0x1111111111111111, LineOffset: 11, Column: 10, IsInlineFrame: true }
+          - { Function: 0x2222222222222222, LineOffset: 22, Column: 20, IsInlineFrame: false }
+        MemInfoBlock:
+          AllocCount:      111
+          TotalSize:       222
+          TotalLifetime:   333
+          TotalLifetimeAccessDensity: 444
+      - Callstack:
+          - { Function: 0x3333333333333333, LineOffset: 33, Column: 30, IsInlineFrame: false }
+          - { Function: 0x4444444444444444, LineOffset: 44, Column: 40, IsInlineFrame: true }
+        MemInfoBlock:
+          AllocCount:      555
+          TotalSize:       666
+          TotalLifetime:   777
+          TotalLifetimeAccessDensity: 888
+    CallSites:
+      - Frames:
+        - { Function: 0x5555555555555555, LineOffset: 55, Column: 50, IsInlineFrame: true }
+        - { Function: 0x6666666666666666, LineOffset: 66, Column: 60, IsInlineFrame: false }
+        CalleeGuids: [ 0x100, 0x200 ]
+      - Frames:
+        - { Function: 0x7777777777777777, LineOffset: 77, Column: 70, IsInlineFrame: true }
+        - { Function: 0x8888888888888888, LineOffset: 88, Column: 80, IsInlineFrame: false }
+        CalleeGuids: [ 0x300 ]
+...