[PATCH] D124750: [MLIR] Add a utility to sort the operands of commutative ops

[River Riddle via Phabricator via cfe-commits]

rriddle added a comment.

+1 on all of the other comments, especially related to the use of strings.

In D124750#3503607 <https://reviews.llvm.org/D124750#3503607>, @Mogball wrote:

> On the matter of whether this should be a canonicalization, my concern with this is that if an operation has its own preferred ordering of operands that conflicts with the sort, then this will cause canonicalization to loop infinitely.
>
> It's not actually the canonicalizer pass that moves constants to the right hand size. It's the folder. And it probably shouldn't be the folder that does this. So I'm open to making this part of canonicalization IF the sorted operand order produced by this utility is the canonical order of operands for commutative operations, so that conflicts are not possible.

We can decide whatever we want the canonical ordering of operands to be for the Commutative trait. We don't have to leave things up to operations if it doesn't make sense.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124750/new/

https://reviews.llvm.org/D124750