srishti-pm added a comment. **`Replying to the various comments given in this revision:`**
**1. Regarding string manipulation:** > I need to look at the algorithm in more detail, but I'm not a fan of using a > string key. Concatenating strings to make compound keys is not very efficient > and potentially brittle. Can you assign unique IDs and use an array of IDs > instead? Sure, I'm currently brainstorming to come up with a way to do this. > +1 to Mehdi's question about just stable sorting based on based on 4 criteria > (3 buckets + ordering within (1)) and then we should be able to avoid all the > string mangling too as Jeff asked about. Actually, that question of Mehdi is now obsolete (I think). It was based on my incorrect documentation of the "sorting rule". I have now corrected the documentation of the rule (in the revision summary, the commit summary, and the code comments). The rule was finalized in a recent RFC (refer to comments starting from here: https://discourse.llvm.org/t/mlir-pdl-extending-pdl-pdlinterp-bytecode-to-enable-commutative-matching/60798/19?u=srishtisrivastava). Does this sound okay? **2. Regarding reversing non-constant-like op and block argument order:** > I would have thought block-arguments would come first as we don't know their > values, while non-constant-like ops could be folded at some point and then > become constant-like. Meaning, they seem closer to constant than block > arguments. Sure. This sounds much better actually. I think at some point during the implementation, I had also thought of the same :) I'll do this. Thanks for the suggestion. 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
