[PATCH] D51362: [GlobalISel][IRTranslator] Canonicalize G_ICMP to have constant operands last

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added a comment.

> Does that mean that something after the last InstCombine produces icmp's with the non-canonical operand order? If so maybe that something needs to be chased down and fixed instead.

That's exactly the point :).

I understand that O0 doesn't run instcombine, thus the problem. It may be a good idea to identify what is canonicalization and what is optimization in instcombine and separate both so that it can be run at O0.

Regarding the solution of doing that in the MIRBuilder, this has preference given all MIR pass can benefit from it. That said, I would like we make a conscious choice of what we put here again to avoid code duplication as much as possible.
Basically what I am saying is we should ask ourselves, assuming the input IR is canonicalized, is this a pattern we may introduce and therefore that we need to clean-up one way or another. I would tend to think that when we create instruction with constant we can discipline ourselves for not putting them on the LHS.

Ideally, we could express the canonicalization rules in a higher-level language and use them both in IR and MIR. Though, we don't want to add such dependency to the progress of GISel.


Repository:
  rL LLVM

https://reviews.llvm.org/D51362