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

[Nikita Popov 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>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/105991 at github.com>


nikic wrote:

Okay, maybe my comment didn't spell it out enough. I'm referring to combines that already exist as canonicalizations in the target-independent optimizer. Of course there are also combines during isel that are target-dependent,  undoing middle-end canonicalizations, or similar. Those are not targeted at legalization artifacts and can be useful pre-legalization.

What I want to push back against here is blindly copying over optimizations from InstSimplify or InstCombine, without any justification for why these optimizations need to be replicated inside GlobalISel. I'm not necessarily saying that the combines proposed here are bad, but they need more justification than "inspired by simplifyICmpInst and simplifyICmpWithZero". You should understand, and be able to articulate to reviewers, why it is beneficial to have these combines replicated in GlobalISel.

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