[PATCH] Add parsing support for discriminators and indirect call samples.

[Diego Novillo]

  I'm not really sure how to update this issue with a new version of the changes I've added. I've got other patches on top of this now. Help?


================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:118
@@ -117,2 +117,3 @@
   /// \brief Total number of samples collected at the head of the function.
+  /// TODO: Use head samples to estimate a cold/hot attribute for the function.
   unsigned TotalHeadSamples;
----------------
Chandler Carruth wrote:
> LLVM uses "FIXME:" comments more often.
Done.

================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:349-352
@@ -347,6 +348,6 @@
 ///    function1:total_samples:total_head_samples:number_of_locations
-///    location_offset_1: number_of_samples
-///    location_offset_2: number_of_samples
+///    offset1[.discriminator]: number_of_samples [fn1:num fn2:num ... ]
+///    offset2: number_of_samples
 ///    ...
-///    location_offset_N: number_of_samples
+///    offsetN: number_of_samples
 ///
----------------
Chandler Carruth wrote:
> I would repeat the optional bits on each line so it doesn't look like they are reserved for the first offset?
Done.

================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:377-389
@@ +376,15 @@
+///
+/// d- [OPTIONAL] Potential call targets and samples. If present, this
+///    line contains a call instruction. This models both direct and
+///    indirect calls. Each called target is listed together with the
+///    number of samples. For example,
+///
+///    130: 7  foo:3  bar:2  baz:7
+///
+///    The above means that at relative line offset 130 there is a
+///    call instruction that calls one of foo(), bar() and baz(). With
+///    baz() being the relatively more frequent call target.
+///
+///    TODO: This is currently unhandled, but it has a lot of
+///          potential for aiding the inliner.
+///
----------------
Chandler Carruth wrote:
> This representation seems really strange to me. I wouldn't mix the two types of information.
> 
> 
> I continue to think it would be best to think of the text format as "record oriented" with one line representing a record, and each record being be written in a "denormalized" form for the purpose of testing. Then we could use, for example:
> 
> caller_mangled_name:130: foo:3 bar:2 baz:7
> 
> This provides all of the information for a single indirect call edge.
> 
> Maybe it would be easiest to add support for this after doing some of the changes to the text file format I asked for in the original review (essentially, moving it to a denormalized line-oriented format to ease test case management long-term).
I'll change the format in the next patch.

The denormalization already happened. It's in http://llvm-reviews.chandlerc.com/D2419

================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:448-453
@@ +447,8 @@
+
+      // TODO: Handle discriminator information (in Matches[2]).
+      //
+      // If we find '.NUM' after the sample count, that indicates a
+      // DWARF discriminator
+      // (http://wiki.dwarfstd.org/index.php?title=Path_Discriminators)
+      // This is currently only emitted by GCC. Simply ignore it for now.
+
----------------
Chandler Carruth wrote:
> I don't think you need to duplicate all of this information.
Done.

================
Comment at: lib/Transforms/Scalar/SampleProfile.cpp:457-474
@@ -410,1 +456,20 @@
+
+      // TODO: Handle called targets (in Matches[4]).
+      //
+      // Now parse call targets. If this line makes function calls,
+      // the target functions will be listed as:
+      //
+      // mangled_1_name:NUM  mangled_name_2:NUM  ...  mangled_name_N:NUM
+      //
+      // This models indirect calls and multiple calls on the same line.
+      //
+      // We do not currently use this data, but we need to handle
+      // its parsing because it can be emitted by the external
+      // profiler.
+      //
+      // It could be used to add weights to the callgraph and
+      // influence the inliner. Additionally, it could be used to
+      // group functions from different files into the same
+      // optimization unit during LTO.
+
       // When dealing with instruction weights, we use the value
----------------
Chandler Carruth wrote:
> Same comment as above -- no need for duplicated information here.
Done.


http://llvm-reviews.chandlerc.com/D2355