[llvm] [GlobalIsel] Visit ICmp (PR #105991)

Thorsten Schütt via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 10 22:59:10 PDT 2024


tschuett wrote:

This combiner combines opcodes that are not coming as legalization artefacts from the legalizer. It is a generic optimizing combiner. We use the combiner to reduce code size and make the code more amenable to execute (G_MUL -> G_ADD). It is not the first time that we benefit from the expertise of the middle-end in analysis and optimizations. The `isKnownNonZero` analysis is established in the middle-end and the icmp x,0 combine shows code size improvements. For reasons completely unknown to me, this PR blows up in comments and a new review style.

I stated that if we change the order of the two undef optimizations the results will change. I am asked to hoist these optimizations into separate combines where I loose control of the order. 

https://github.com/llvm/llvm-project/pull/90618 blindly copied code from the middle-end with almost unnoticeable code size improvement and without cost benefit analysis. It wasn't run on a set of benchmarks and there was no compile-time analysis.

I mentioned above `v_cmp_eq_u32_e64 s[4:5], 0, v5`. We need  canonicalization of constants on icmps to the RHS and the icmp x, 0 combine to play to together. At the same time, I have requested changes:
"This needs to be split up into separate PRs, where each only adds a single fold, and each fold is well justified."

I am asked to separate canonicalization and the icmp x, 0 combine into two separate PRs. Neither of which is profitable in isolation. I am stuck again. I am asked to do tasks that I cannot do?!? 

https://github.com/llvm/llvm-project/pull/105991


More information about the llvm-commits mailing list