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

[Amara Emerson via llvm-commits]

Thorsten =?utf-8?q?Schütt?= <schuett at gmail.com>,
Thorsten =?utf-8?q?Schütt?= <schuett at gmail.com>,
Thorsten =?utf-8?q?Schütt?= <schuett at gmail.com>,
Thorsten =?utf-8?q?Schütt?= <schuett at gmail.com>,
Thorsten =?utf-8?q?Schütt?= <schuett at gmail.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/105991 at github.com>


aemerson wrote:

> I strongly disagree with the change request. Compares and only compares deserve special treatment.
Why only compares? Why not G_BRCOND or G_PTR_ADD?

> Placing several optimizations into one function is beneficial.

Yes, agreed there are trade-offs. For example, if we can improve the speed of the combiner by 20% by having merging some G_ICMP combines then of course that's a very compelling reason to compromise our convention. I don't see any hard evidence that there is such a gain to be made.

> They share resources and order matters.

You've said this "order matters" issue before but I've yet to see an explanation of *what* exactly goes wrong if you split them? And as for resource sharing, well the only non-trivial thing I see being shared in visitICMP is `MIRHS` which is shared between the 2nd and 3rd combines.

> second week ping

You won't even rename the function which I requested as the bare minimum for this patch to be considered.

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