[PATCH] D95962: [CSSPGO] Introducing dangling pseudo probes.

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineInstr.h:1812-1814
+    uint32_t OrigAttr = AttrOperand.getImm();
+    OrigAttr |= (uint32_t)Attr;
+    AttrOperand.setImm(OrigAttr);
----------------
wmi wrote:
> Nit: AttrOperand.setImm(AttrOperand.getImm() | (uint32_t)Attr);
Sounds good.


================
Comment at: llvm/lib/CodeGen/PseudoProbeInserter.cpp:87
+        auto MII = MBB.rbegin();
+        while (MII != MBB.rend()) {
+          // Skip all pseudo probes followed by a real instruction since they
----------------
wmi wrote:
> Can it stop when MII equals to FirstInstr? Otherwise will the moved pseudo probe be removed and inserted before FirstInstr again?
Yes, it can stop. Note that `FirstInstr` is not a probe but a real instruction. When MII equals to it, the `break` below will be run. 


================
Comment at: llvm/lib/CodeGen/PseudoProbeInserter.cpp:110
+          if (MI.isPseudoProbe())
+            MI.addPseudoProbeAttribute(PseudoProbeAttributes::Dangling);
+        }
----------------
wmi wrote:
> It set all the pseudo probes in MBB to dangling. For DirectCall/IndirectCall types of pseudo probe, they are used to represent inline stack. Do they need to be set to dangling?
DirectCalls/IndirectCalls are real call instructions and can be sampled correctly. If there are calls in the block, other types of probes will not be considered dangling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95962