neat! lgtm

Tried out the change on compile-time tracker, seems very neutral: http://llvm-compile-time-tracker.com/compare.php?from=2e8ef93439a4a430fe5f96b9d999ce96405345f4&to=c4b2e5550264027608d40db0da53b1b9019d45a2&stat=instructions:u

gen test with update_test_checks

feedback

@efriedma , is this patch good to go? Thanks!

In D145463#4175819, @AlexM wrote:

In D145463#4175715, @mtrofin wrote:

Would adding an assert(Callee && "Expected to be non-null due to check at the start of the loop") (or something like that) above the if-check make Coverity happy?

Either removing the check completely or moving it from the if to an assert will make Coverity happy. Would the assert be preferable to complete removal from a stylistic/readability perspective?

more clarity

In D145463#4175673, @AlexM wrote:

I'm very new to this and trying to get my toes wet in the review process with some very minor non-controversial changes. Please let me know if I've made any mistakes in the process.

clarify it's not about indirect calls only

I *think* @rnk is out for a few weeks, @nikic, does the patch look reasonable to you?

handle cycles

<eyeroll>

remove spurious member

non-virt

propagate

newline at end of test file

In D143948#4124802, @wenlei wrote:

It's a bit surprising that issues like this went unnoticed for so long... Thanks for the fix.

feedback

In D143311#4120232, @thakis wrote:

The enable of this test without tflite broke tests on Mac: http://45.33.8.238/macm1/54722/step_11.txt

Please take a look and revert for now if it takes a while to fix.

(Saying this here since the other change doesn't have a phab link in its commit message.)

In D143311#4120232, @thakis wrote:

The enable of this test without tflite broke tests on Mac: http://45.33.8.238/macm1/54722/step_11.txt

Please take a look and revert for now if it takes a while to fix.

(Saying this here since the other change doesn't have a phab link in its commit message.)

In D143624#4116546, @aeubanks wrote:

Had a chat offline with @mtrofin, wanted to be clear for future purposes that we do need the separate AlwaysInliner pass because it's used in -O0 and constructing a call graph there is non-trivial in terms of compile time.

lgtm. Like @aeubanks was saying, let's just give a bit of time (1 month or so?) between when this lands and until we clean up the "mandatory" notion from the advisor, just to make sure nothing breaks/regresses.

In D143624#4115986, @aeubanks wrote:

__clang_hip_math.hip is annoying...

We'll need to remove the MandatoryFirst inliner in ModuleInlinerWrapperPass, although not sure if @mtrofin has any issues with that or not

Thanks for doing this, looks so much simpler - basically ready to go imho after addressing the error handling comment

rebase & feedback

You'll need to update the failing pass structure tests, when you do, can you put a comment there (in the tests) that this pass must be right before asmprinter to make sure nothing mutates MBB freqs?

cloned 2022->2023 entry, relabeled ID

feedback

In D143218#4104421, @dyung wrote:

There still seems to be failures related to this change, can you take a look please?

https://lab.llvm.org/buildbot/#/builders/247/builds/1260

feedback