mehdi_amini added a comment.
Seems like this should be added to canonicalization? The "push constants to the
right hand side" is there already.
I also don't understand the complexity of the implementation, I may need an
example to understand why you're recursively operating on the producer ops for
the operands.
From the revision description: `, (1) the operands defined by non-constant-like
ops come first, followed by (2) block arguments, and these are followed by (3)
the operands defined by constant-like ops` which all seems like a fairly local
check: a stable-sort on the operands would be deterministic and local to a
single operation.
================
Comment at: mlir/lib/Transforms/Utils/CommutativityUtils.cpp:235
+ bfsOfOperand->key = (Twine(bfsOfOperand->key) + Twine("1") +
+
std::string(frontAncestor->getName().getStringRef()))
+ .str();
----------------
The string conversion seems unnecessary to me?
================
Comment at: mlir/lib/Transforms/Utils/CommutativityUtils.cpp:271
+ for (Operation *operandDefOp : operandDefOps) {
+ OperandBFS *bfsOfOperand = new OperandBFS();
+ bfsOfOperand->pushAncestor(operandDefOp);
----------------
Is this a leak?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124750/new/
https://reviews.llvm.org/D124750
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
