[PATCH] D120295: [SampleProfile] [ICP] Handle the case when the option `MaxNumPromotions` is zero.

Mingming Liu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 22 15:29:47 PST 2022


luna added a comment.

In D120295#3338540 <https://reviews.llvm.org/D120295#3338540>, @davidxl wrote:

> Is it possible to check this in the caller context?

Done.

In D120295#3338813 <https://reviews.llvm.org/D120295#3338813>, @xur wrote:

> I think the reason MaxNumPromotion==0 breaks is likely the 0 size array think (like MaskRay mentioned).
>
> Explicitly checking of 0 is a good fix here.
>
> LGTM except I'm not sure if the second check is needed.

Yeah I realized `doesHistoryAllowICP` will return false, and `updateIDTMetadata` won't be executed within `tryPromoteAndInlineCandidate`.

There is another caller of  `updateIDTMetadata` (https://github.com/llvm/llvm-project/blob/606cb8548a1b7763e0c8489c5efe66803a7ede72/llvm/lib/Transforms/IPO/SampleProfile.cpp#L1640), so keep the value check within `updateIDTMetadata`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120295



More information about the llvm-commits mailing list