[PATCH] D103041: [SampleFDO] New hierarchical discriminator for FS SampleFDO (ProfileData part)

[Hongtao Yu via Phabricator via llvm-commits]

hoy accepted this revision.
hoy added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/include/llvm/Support/Discriminator.h:17
+#include "llvm/Support/Error.h"
+#include <array>
+#include <assert.h>
----------------
Nit: this is not needed anymore.


================
Comment at: llvm/lib/ProfileData/SampleProfReader.cpp:312
+      if (ProfileIsFS)
+        Discriminator &= getDiscriminatorMask();
+
----------------
xur wrote:
> hoy wrote:
> > Assuming the sample loader runs in multiple places of the pipeline, does it make sense to assert the length of `Discriminator` matches the length of the mask? IIUC, if the number of valid bits of `Discriminator` is more than that of the mask, it means we are loading the profile of an earlier stage and we should use a longer mask?
> I'm not sure if I understand the question.
> Hers the "discriminator" is the one from profile which can have all the discriminator bits (i.e. including the later passes).  Here we use the mask to remove the later bits and merge them together.
You are right. I was confused.


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

https://reviews.llvm.org/D103041