Merge pull request #1096 from reactorlabs/reportChanges

better change tracking in constantfold and gvn

Author: o-

rir/src/compiler/opt/constantfold.cpp

@@ -46,7 +46,7 @@ SEXP qt(SEXP c, Preserve& p) {
                     auto res = Rf_eval(                                        \
                         p(Rf_lang3(Operation, qt(lhs, p), qt(rhs, p))),        \
                         R_BaseEnv);                                            \
-                    anyChange = true;                                          \
+                    iterAnyChange = true;                                      \
                     instr->replaceUsesWith(cmp.module->c(res));                \
                     next = bb->remove(ip);                                     \
                 }                                                              \
@@ -154,9 +154,7 @@ static bool isStaticallyNA(Value* i) {
 bool Constantfold::apply(Compiler& cmp, ClosureVersion* cls, Code* code,
                          LogStream& log, size_t iteration) const {
     EarlyConstantfold cf;
-    cf.apply(cmp, cls, code, log, iteration);
-
-    bool anyChange = false;
+    bool anyChange = cf.apply(cmp, cls, code, log, iteration);
 
     Preserve p;
     std::unordered_map<BB*, bool> branchRemoval;
@@ -1056,6 +1054,7 @@ bool Constantfold::apply(Compiler& cmp, ClosureVersion* cls, Code* code,
     }
     for (auto bb : dead) {
         assert(!reachable.count(bb));
+        anyChange = true;
         delete bb;
     }
 

rir/src/compiler/opt/gvn.cpp

@@ -12,6 +12,7 @@ namespace pir {
 
 bool GVN::apply(Compiler&, ClosureVersion* cls, Code* code, LogStream& log,
                 size_t) const {
+    bool changed = false;
     std::unordered_map<size_t, SmallSet<Value*>> reverseNumber;
     std::unordered_map<Value*, size_t> number;
     {
@@ -213,6 +214,8 @@ bool GVN::apply(Compiler&, ClosureVersion* cls, Code* code, LogStream& log,
                 auto i = Instruction::Cast(*p);
                 p++;
                 if (i && i != firstInstr) {
+                    if (i->bb() != firstInstr->bb())
+                        changed = true;
                     if (!firstInstr->type.isA(i->type))
                         continue;
                     if (i->bb() == firstInstr->bb()) {
@@ -269,10 +272,7 @@ bool GVN::apply(Compiler&, ClosureVersion* cls, Code* code, LogStream& log,
     // Sometimes a dead instruction will trip the verifier.
     BBTransform::removeDeadInstrs(code, 1);
 
-    // The current implementation of GVN almost always finds something to
-    // change. We use the changed flag to determine when to stop optimizing and
-    // it is thus just too noisy.
-    return false;
+    return changed;
 }
 
 } // namespace pir