In D100016#2678789, @skatkov wrote:

@thegameg, I landed NFC before to move from if-else to if-continue. Is it readable now or you still want me to separate added logic into utility function?

In D100017#2673209, @skatkov wrote:

In D100017#2673208, @thegameg wrote:

Is this really useful as a user-surfacing remark? Won't it have a really high false positive rate due to coalescing?

To be honest - not sure. But if RA can assigned physical register and can eliminate copy it a good sign.
So probably I also need to check reg map whether physical registers are different or not... Need to check...

Is this really useful as a user-surfacing remark? Won't it have a really high false positive rate due to coalescing?

This looks good too, thanks! Please add a test or extend the ones in llvm/test/CodeGen/AArch64/arm64-spill-remarks.ll.

In D100012#2673146, @skatkov wrote:

In D100012#2673142, @thegameg wrote:

LGTM. Doesn't seem like there are any tests testing this remark, maybe you can add one if you have something handy?

There are some tests
./CodeGen/AArch64/arm64-spill-remarks-treshold-hotness.ll
./CodeGen/AArch64/arm64-spill-remarks.ll

I'll probably add something like this.

LGTM. Doesn't seem like there are any tests testing this remark, maybe you can add one if you have something handy?

Address comments + comments in header.

Thank you!

Add StoreVolatile: false and StoreAtomic: false under ExtraArgs.

LGTM. Not sure if there's a cleaner way.

LGTM, nice!

LGTM

LGTM

Looks good, thanks!

Looks great, thanks!

In D89892#2349184, @anemet wrote:

How about adding a total count as well? That should help quickly eye ball the ratios.

This looks great! Thanks!

In rG0345d88de654259ae90494bf9b015416e2cccacb#953264, @hgreving wrote:

Is this really necessary to remove? For example, our backend is using EntrySU edges for top edges from the basic block entry, in order to account for clearances. It seems to make sense to have a node like this.

I am totally fine with keeping this in-tree, even unused, if it helps several out-of-tree backends. Thanks for speaking up.

LGTM

Can you wrap the condition around the whole file? The rest is not useful without the library anyway.

Won't be back to this anytime soon.

LGTM

This looks great, thanks! Since it's a CodeGen pass, I would suggest using a MIR test instead of an IR one, but I think it's probably easier to read it as it is right now.

In D85809#2218742, @tejohnson wrote:

Just to clarify, I wasn't suggesting putting it in another patch, but just bringing it back here (plus adding in the gold-plugin support). The other patch I was referring to was the follow on patch D85810 where I also suggested adding support to gold-plugin.cpp.

In D85809#2217341, @weiwang wrote:

I have removed the change to llvm-lto

LGTM, thanks for plugging all these options through!

Hi @compnerd, it looks like this fails to link: http://green.lab.llvm.org/green/job/clang-stage1-RA/13171/console

In D84690#2179205, @dblaikie wrote:

Indeed, sorry that fell through & I didn't understand enough about iterators to instead suggest falling back to an input iterator instead of figuring out how to fit the semantics of the forward iterator.

I had an attempt here: https://reviews.llvm.org/D33419, I think similar concerns came up in that review.

In D84207#2164754, @jroelofs wrote:

Not sure why our public bots didn't catch it.

In D81645#2160087, @dantrushin wrote:

In D81645#2159907, @thegameg wrote:

Hi, I see CodeGen/X86/statepoint-vreg.mir failing with UBSAN enabled on our internal bots:

Should be fixed already.
Does your sources include e04fe9aefd4?

Hi, I see CodeGen/X86/statepoint-vreg.mir failing with UBSAN enabled on our internal bots:

LGTM, thanks!