[PATCH] D147286: [FS-AFDO] Assign discriminators to pseudo probes

[Hongtao Yu via Phabricator via llvm-commits]

hoy added a comment.

In D147286#4242407 <https://reviews.llvm.org/D147286#4242407>, @wenlei wrote:

>> Also call instructions are excluded since their dwarf discriminators are used for other purposes, i.e, storing probe ids.
>
> How/where do we handle the FS discriminator for call probes? Or will that be handled in a separate patch?

They'll be handled separately. Basically they will be ignored in MIR sample loading.



================
Comment at: llvm/lib/CodeGen/MIRFSDiscriminator.cpp:134-140
+      if (HasPseudoProbe) {
+        // Only assign discriminators to pseudo probe instructions. Call
+        // instructions are excluded since their dwarf discriminators are used
+        // for other purposes, i.e, storing probe ids.
+        if (!I.isPseudoProbe())
+          continue;
+      } else if (ImprovedFSDiscriminator && I.isMetaInstruction()) {
----------------
wenlei wrote:
> hoy wrote:
> > wenlei wrote:
> > > nit: 
> > > ```
> > > if ((ImprovedFSDiscriminator && I.isMetaInstruction()) || 
> > >     (HasPseudoProbe && !I.isPseudoProbe()) {
> > >   continue;
> > > }
> > > ```
> > Actually we need to go with the probe check first because probe is also a meta instruction.
> > 
> > 
> > 
> Ok, but when HasPseudoProbe is false, are we still going to have probe instructions? I thought we won't.
Right, when `HasPseudoProbe` is false we don't have probes. But `ImprovedFSDiscriminator` and `HasPseudoProbe` can be true at the same time, so checking  `ImprovedFSDiscriminator && I.isMetaInstruction())` first will ignore all probe instructions. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147286