[PATCH] D124490: [InstrProf] Minimal Block Coverage

Shoaib Meenai via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 29 08:29:05 PDT 2023


smeenai added a comment.

In D124490#4230857 <https://reviews.llvm.org/D124490#4230857>, @kyulee wrote:

> This looks good to me as this change is effective even for large programs.
> This good change has been sitting for a while. I think it would be beneficial for the community to have it, so I approve it.
> I also suggest following up any concerns and feedbacks after it lands.

Just to clarify, as far as I can tell, all other comments have been addressed, and the one remaining concern is around using a faster algorithm. I believe that concern is mitigated by applying the size cutoff. You could argue that having the linear algorithm is cleaner than having a quadratic one with a cutoff, but it sounds like the linear algorithm would be significantly more complex, which is also a trade-off. For the intended application, where being optimal for size is really important and you're unlikely to have a function large enough to hit the cutoff in practice, going with the simpler algorithm seems like a reasonable trade-off.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124490



More information about the llvm-commits mailing list