[PATCH] D71448: [Legalizer] Making artifact combining order-independent

[Roman Tereshin via Phabricator via llvm-commits]

rtereshin marked 2 inline comments as done.
rtereshin added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h:481
+
+    SmallVector<Register, 4> UpdatedDefs;
+    bool Changed = false;
----------------
paquette wrote:
> Can we have a comment explaining what the purpose of `UpdatedDefs` is?
> 
> E.g. "Store the refs that were changed by a combine, so that we can..." 
Sure! I'll come up with something shortly.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h:507
     }
+    while (!UpdatedDefs.empty()) {
+      Register NewDef = UpdatedDefs.pop_back_val();
----------------
paquette wrote:
> A comment explaining what you're doing here at a high level would be nice too.
Not entirely sure how would I shape it. You mean repeating all the same stuff described in the commit message and the test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71448