Ah, I am sorry. Thanks for fixing it.

In D85808#2424588, @tejohnson wrote:

lgtm with a couple of minor nits noted below that you can fix before submitting

1. Fix typo.
2. Minor order adjustment in testcase.

rebase

rebase

@tejohnson @MaskRay Do you have other comments?

@tejohnson @MaskRay Do you have other comments?

In D85808#2403587, @tejohnson wrote:

Thanks for adding the Driver test. I was thinking of something to test the CompilerInvocation changes, similar to your test using opt, that ensures the option has the desired behavior when invoked via clang. Looks like there is an existing test clang/test/Frontend/optimization-remark-with-hotness.c that perhaps could be extended or leveraged?

1. Add clang test with remarks output.
2. Fix a missing dependency on PSI in legacy pass manager.

revert back MSVC fix

update to address MSVC build error

update test case

update test case for clang option pass-through

Thanks! I will wait for his input.

@MaskRay @tejohnson, do you have other comments regarding this change and its dependent?

add test case

Diff was committed, but did not close automatically. Manual close it now.

fix typo

Sorry for the long delay. Got held up by some other task.

In D89341#2331338, @craig.topper wrote:

In D89341#2328996, @weiwang wrote:

Thanks for the comment, Craig.

isSExtAbsoluteSymbolRef does seem to check for the width of immediate. If the immediate can be encoded directly, the node should be replaced with one of the SUB64ri* nodes. I think the X86Wrapper node can be replaced with a corresponding imm node if conditions are met, then the matching can proceed.

I was thinking that if we don't have range information from getAbsoluteSymbolRange, but the Width passed to isSExtAbsoluteSymbolRef is 32 and the code model is small we could return true?

In D89341#2328984, @wenlei wrote:

Would be good to share benchmark/performance numbers you have for this change too. Thanks.

Thanks for the comment, Craig.

Thanks @MaskRay ! I am really sorry I didn't catch up your comments earlier. My email somehow stops giving me notifications on diff updates.

Address Comments.

@tejohnson @MaskRay Do you have other comments to the change?

@tejohnson @MaskRay Do you have other comments to the change?

minor update to test case

1. remove unreachable code
2. udpate test case

update test case

Add test case for indirect call. Without the fix, the test will crash.

Really appreciate the feedbacks. If there is no more, could anyone approve 3 changes? Thanks.

remove redundant check

try to fix windows build linking error.

try to fix windows build error. Use fully qualified name for enum value Optional.

add comment on test case.

fix typo

coding style change

Try to fix a windows build error. No msvc available locally, so even with this change, the build could still fail.

move some unrelated change into parent diff.

update:

1. reorganize code splitting with parent diff.
2. format and style change.

update:

1. move internal remarks hotness threshold type change and its related handling code from child diff into this one. With this move, remarks hotness threshold now accepts 'auto' value, but its handling is currently disabled.
2. format change.

Instead of adding checks for all passes/analyses, use light-weight ORE to emit remarks for dead functions.

update:

1. add --plugin-opt alias for remarks in lld
2. handle both lld and ld.gold pass-through
3. simplify target checking logic
4. code format update

format update

format update

update:
add remarks hotness threshold to all the tools uses lto::setupLLVMOptimizationRemarks

In D85809#2214246, @tejohnson wrote:

The changes to *LTOCodeGenerator and the test using llvm-lto are unrelated to lld (they are the old LTO API used by ld64). So they should be moved to a different patch or the patch name changed. Would be good to add llvm-lto2 tests which exercise the new LTO API changed in LTO.cpp, LTOBackend.cpp etc. Note those changes are broader than just lld (also used by gold-plugin).

changes:

1. revert the change made to llvm-lto.
2. add the new option to llvm-lto2 wit test cases.

In D85808#2214272, @tejohnson wrote:

In 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.

update test case

clang-format

Diff has been split into 3 smaller ones:

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

This is the 2nd 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