[PATCH] D97481: [CSSPGO] Unblocking optimizations by dangling pseudo probes.

[Hongtao Yu via Phabricator via llvm-commits]

hoy marked an inline comment as done.
hoy added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineBasicBlock.h:675-677
+  /// machine code. We try to use the function to skip both debug instructions
+  /// and pseudo probe operations to avoid API proliferation. This should work
+  /// most of the time when considering optimizing the rest of code in the
----------------
wmi wrote:
> > This should work most of the time when ..., except for certain cases ...
> 
> It sounds like "SkipPseudoOp = true" is the more common cases and only certain cases will use the default value. It will lead to the confusion whether user should use the default value or not for now. I know you mentioned you would flip the default value in the future. If you think user is better to choose "SkipPseudoOp = true" in common cases, it may be good to suggest that in the front and add a TODO for flipping the default value.  
Sure, comment added. I'd like to turn it on by default after a few more rounds until all optimizations that could hurt profile quality are understood.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:538-539
 
+  // Ignore danling probes since they are logically deleted and should do
+  // not consume any profile samples.
+  if (Probe->isDangling())
----------------
wmi wrote:
> Nit: should not consume
Fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97481