[PATCH] D124775: [IRSim] Removing check that caused early matching of commutative instructions

Mon May 2 10:41:12 PDT 2022

paquette accepted this revision.
paquette added a comment.
This revision is now accepted and ready to land.

I think this is fine. I have a couple nits on the comments, but I won't hold up review over them.

================
Comment at: llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp:2622

+// This test ensures that when the first instruction in a sequence is
+// a commutative instruction with the same value, but the corresponding
----------------
I think this comment would be clearer if you said "ensure that <instruction in test> is not mapped the same as <other instruction in test>"; it's not clear to me what's going on here right now.

================
Comment at: llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp:2629
+                            entry:
+                              %mul44 = mul i64 undef, undef
+                              %add = add i64 %mul44, %v_33
----------------
This is minor, but it would be really useful to have names for the variables here which describe what they're doing if possible.

E.g.

```
%commutative_inst = mul i64 undef, undef
...
```

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124775