[GlobalMerge][NFC] Skip sorting by profitability when it is not needed

[michaelmaitland]

We were previously sorting by profitability even if we were choosing to merge all globals together, which is not impacted by UsedGlobalSet order.

We can also remove iteration of UsedGlobalSets in reverse order in both cases. In the first csae, the order does not matter. In the second case, we just sort by the order we need instead of sorting in the opposite direction and calling reverse.

This change should only be an improvement on compile time. I have not measured it, but I think it would never make things worse.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[wangpc-pp]

Make sense, LGTM!

[jyknight]

This was not NFC: it causes the comparator not have strict-weak-ordering, because comp(a, b) && comp(b, a) can now be true. Most immediately, this triggers an assert with the test llvm/test/CodeGen/AArch64/global-merge-group-by-use.ll when run under a hardened stdlib (assertion !__comp_(__l, __r) failed: Comparator does not induce a strict weak ordering).

I believe you meant to use > instead of >=.

[michaelmaitland]

This was not NFC: it causes the comparator not have strict-weak-ordering, because comp(a, b) && comp(b, a) can now be true. Most immediately, this triggers an assert with the test llvm/test/CodeGen/AArch64/global-merge-group-by-use.ll when run under a hardened stdlib (assertion !__comp_(__l, __r) failed: Comparator does not induce a strict weak ordering).

I believe you meant to use > instead of >=.

The assertion failure didn’t show up in pre-commit CI and I haven’t gotten any build bot failures. So thanks for pointing it out. Can you please revert or make the change to > for me as I am currently AFK for the night?

Thanks.

[topperc]

It's still not NFC with >. stable_sort won't change the order of equal elements. If there are multiple equal elements reverse would visit the later ones first. Removing the reverse means the earliest equal element first.