[PATCH] D85808: [remarks] Optimization remarks hotness filtering from profile summary

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D85808#2214272 <https://reviews.llvm.org/D85808#2214272>, @tejohnson wrote:

> In D85808#2212297 <https://reviews.llvm.org/D85808#2212297>, @weiwang wrote:
>
>> This is the 3rd of 3 dependent patches:
>>
>> 1. [lld] Enable remarks hotness filtering in lld: https://reviews.llvm.org/D85809
>> 2. [clang] Pass-through remarks options to lld: https://reviews.llvm.org/D85810
>> 3. [remarks] Optimization remarks hotness filtering from profile summary: https://reviews.llvm.org/D85808
>
> You can relate these as Parent/Child patches in Phabricator. See "Edit Related Revisions" on the top right. It helps reviewers to see the relationship and keep track of what patches are still open.
>
> Note these relationships get set up automatically when you upload a patch with "Depends on Dxxxxx." in the description.

Thanks for the tip.



================
Comment at: lld/ELF/Config.h:280
+  // If threshold option is not specified, it is disabled by default.
+  llvm::Optional<uint64_t> optRemarksHotnessThreshold = 0;
 
----------------
tejohnson wrote:
> Since this field is being added in patch D85809 as an unsigned, why not add it as llvm::Optional<> there to start with instead of adding and then changing?
> 
> Ditto for a number of other places in this patch.
Thanks for the feedback! It does seem awkward. When doing the splitting, I tried to make the 3 patches look more self-contained from each other. The `Optional` type change seems unrelated with the first patch. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808